Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1094: Transform CSS selectors according to sanitizer HTML element to AMP component conversions #1175

Merged
merged 17 commits into from May 31, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+97 −35
Diff settings

Always

Just for now

Next

Initial take on CSS selector conversions according to AMP.

  • Loading branch information...
miina committed May 25, 2018
commit e530045a6e8daae4406213b6e51436f549b0a7cf
@@ -1408,6 +1408,88 @@ private function finalize_styles() {
}
}
/**
* Check if CSS selector should be included to style.
*
* @param string $dynamic_selector_pattern Selector pattern.
* @param array $parsed_selector Array of tag and classes.
* @param string $selector CSS selector.
* @return bool If should include CSS selector.
*/
private function should_include_selector( $dynamic_selector_pattern, $parsed_selector, $selector ) {
$dom = $this->dom;
return (
( $dynamic_selector_pattern && preg_match( $dynamic_selector_pattern, $selector ) )
||
(
// If all class names are used in the doc.
(
empty( $parsed_selector['classes'] )
||
0 === count( array_diff( $parsed_selector['classes'], $this->get_used_class_names() ) )
)
&&
// If all IDs are used in the doc.
(
empty( $parsed_selector['ids'] )
||
0 === count( array_filter( $parsed_selector['ids'], function( $id ) use ( $dom ) {
return ! $dom->getElementById( $id );
} ) )
)
&&
// If tag names are present in the doc.
(
empty( $parsed_selector['tags'] )
||
0 === count( array_diff( $parsed_selector['tags'], $this->get_used_tag_names() ) )
)
)
);
}
/**
* Ampify CSS selectors.
*
* @param array $parsed_selector Array of tag and classes.
* @param string $selector CSS selector.
* @return array|bool False if not ampification needed, ampified selector otherwise.
*/
private function get_ampified_selectors_parsed( $parsed_selector, $selector ) {
// @todo Add mappings.
$mappings = array(
'img' => 'amp-img',
);
$ampified_selector = $selector;
$tags = array();
if ( ! empty( $parsed_selector['tags'] ) ) {
foreach ( $parsed_selector['tags'] as $tag ) {
if ( ! array_key_exists( $tag, $mappings ) ) {
$tags[] = $tag;
continue;
} else {
$tags[] = $mappings[ $tag ];
$replace_from = '/(^|>|~|\s)' . $tag . '\b/';
$replace_to = '\1' . $mappings[ $tag ];
$ampified_selector = preg_replace( $replace_from, $replace_to, $ampified_selector );
}
}
}
if ( $selector === $ampified_selector ) {
return false;
}
return array(
'selector' => $ampified_selector,
'parsed_selector' => array(
'classes' => isset( $parsed_selector['classes'] ) ? $parsed_selector['classes'] : array(),
'tags' => $tags,
'ids' => isset( $parsed_selector['ids'] ) ? $parsed_selector['ids'] : array(),
),
);
}
/**
* Finalize a stylesheet set (amp-custom or amp-keyframes).
*
@@ -1446,51 +1528,31 @@ function( $selector ) {
$stylesheet_set['processed_nodes'] = array();
$final_size = 0;
$dom = $this->dom;
foreach ( $stylesheet_set['pending_stylesheets'] as &$pending_stylesheet ) {
$stylesheet = '';
foreach ( $pending_stylesheet['stylesheet'] as $stylesheet_part ) {
if ( is_string( $stylesheet_part ) ) {
$stylesheet .= $stylesheet_part;
} else {
list( $selectors_parsed, $declaration_block ) = $stylesheet_part;
if ( $should_tree_shake ) {
$selectors = array();
foreach ( $selectors_parsed as $selector => $parsed_selector ) {
$should_include = (
( $dynamic_selector_pattern && preg_match( $dynamic_selector_pattern, $selector ) )
||
(
// If all class names are used in the doc.
(
empty( $parsed_selector['classes'] )
||
0 === count( array_diff( $parsed_selector['classes'], $this->get_used_class_names() ) )
)
&&
// If all IDs are used in the doc.
(
empty( $parsed_selector['ids'] )
||
0 === count( array_filter( $parsed_selector['ids'], function( $id ) use ( $dom ) {
return ! $dom->getElementById( $id );
} ) )
)
&&
// If tag names are present in the doc.
(
empty( $parsed_selector['tags'] )
||
0 === count( array_diff( $parsed_selector['tags'], $this->get_used_tag_names() ) )
)
)
);
if ( $should_include ) {
$selectors = array();
foreach ( $selectors_parsed as $selector => $parsed_selector ) {
if ( $should_tree_shake ) {
if ( $this->should_include_selector( $dynamic_selector_pattern, $parsed_selector, $selector ) ) {
$selectors[] = $selector;
}
} else {
// Lets map the amp elements and replace the correct one and then check if we should include the replacement.
$amp_selectors_parsed = $this->get_ampified_selectors_parsed( $parsed_selector, $selector );
if ( is_array( $amp_selectors_parsed ) && $this->should_include_selector( $dynamic_selector_pattern, $amp_selectors_parsed['parsed_selector'], $amp_selectors_parsed['selector'] ) ) {
// @todo Check if there could be selector's array?
$selectors[] = $amp_selectors_parsed['selector'];
} else {
$selectors = array_keys( $selectors_parsed );
}

This comment has been minimized.

Copy link
@miina

miina May 25, 2018

Author Collaborator

@westonruter Started adding the logic within this method since it seemed to be related with tree shaking in terms of adding/not adding the selector if it's not found after replacing. That's still WIP (e.g. currently works only in case ! $should_tree_shake and is missing most of the mappings) but let me know if there are any concerns with this approach.

This comment has been minimized.

Copy link
@miina

miina May 25, 2018

Author Collaborator

Hmm, maybe one potential issue could be that the size of the style could exceed the limit if doing the replacement here.

This comment has been minimized.

Copy link
@miina

miina May 28, 2018

Author Collaborator

Moving the logic to process_css_declaration_block to avoid issues with style size changing after the size check.

}
} else {
$selectors = array_keys( $selectors_parsed );
}
if ( ! empty( $selectors ) ) {
$stylesheet .= implode( ',', $selectors ) . $declaration_block;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.