Skip to content

Commit

Permalink
Merge branch 'develop' of github.com:ampproject/amp-wp into add/rewri…
Browse files Browse the repository at this point in the history 
…te-amp-urls-transformer-from-library

* 'develop' of github.com:ampproject/amp-wp: (26 commits)
  Remove unused php use statements
  Update DetermineHeroImagesTest to use amp-img instead of img
  Prevent DetermineHeroImages transformer from targeting any img element
  Avoid identifying noscript fallback img as hero image candidates
  Bump eslint-plugin-jest from 24.3.2 to 24.3.4
  Bump postcss from 8.2.8 to 8.2.9
  Bump svgo from 2.2.2 to 2.3.0
  Bump @babel/core from 7.13.10 to 7.13.14
  Bump eslint from 7.22.0 to 7.23.0
  Bump classnames from 2.2.6 to 2.3.1
  Add covers phpdoc tags
  Fix tests and optimize attribute removal
  Restore object-fit:contain for a reduced-specificity selector
  Omit problematic styling attributes from noscript fallback elements
  Fix inheritance of replaced-content properties on noscript fallbacks
  Fix double arrow alignment
  Remove obsolete core sanitizer CSS code from 2019, 2017, and 2014
  Inherit replaced-content properties for replaced content shadow elements and noscript fallbacks
  Remove obsolete object-fit:contain for unknown-sized image since redundant with object-fit=contain attr
  Bump postcss-import from 14.0.0 to 14.0.1
  ...
  • Loading branch information
@westonruter
westonruter committed Apr 9, 2021
2 parents 08aeebf + 81b9df7 commit 80cf883
Show file tree
Hide file tree
Showing 21 changed files with 281 additions and 402 deletions.
28 changes: 21 additions & 7 deletions assets/css/src/amp-default.css
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@

/*
* Prevent cases of amp-img converted from img to appear with stretching by using object-fit to scale.
* Prevent cases of amp-img converted from img to appear with stretching by using object-fit to scale. Only do this for
* amp-img/amp-anim elements that we converted from <img> via AMP_Img_Sanitizer. This is key for images that get 100%
* width such as in the post content so that the contents do not get stretched or cropped.
* See <https://github.com/ampproject/amphtml/issues/21371#issuecomment-475443219>.
* Also use object-fit:contain in worst case scenario when we can't figure out dimensions for an image.
* Additionally, in side of \AMP_Img_Sanitizer::determine_dimensions() it could $amp_img->setAttribute( 'object-fit', 'contain' )
* so that the following rules wouldn't be needed.
*/
amp-img.amp-wp-enforced-sizes[layout="intrinsic"] > img,
amp-anim.amp-wp-enforced-sizes[layout="intrinsic"] > img {
.amp-wp-enforced-sizes {
object-fit: contain;
}

Expand Down Expand Up @@ -89,3 +86,20 @@ amp-carousel .amp-wp-gallery-caption {
button[overflow] {
bottom: 0;
}

/*
* Ensure relevant properties for "replaced elements" which are set on sanitizer-converted custom elements will be
* inherited to the actual replaced element which is the shadow element and also to the noscript fallback element.
*/
amp-anim img,
amp-anim noscript,
amp-iframe iframe,
amp-iframe noscript,
amp-img img,
amp-img noscript,
amp-video video,
amp-video noscript {
image-rendering: inherit;
object-fit: inherit;
object-position: inherit;
}
10 changes: 0 additions & 10 deletions includes/amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,16 +480,6 @@ function amp_is_available() {
return false;
}

/*
* If this is a URL for validation, and validation is forced for all URLs, return true.
* Normally, this would be false if the user has deselected a template,
* like by unchecking 'Categories' in 'AMP Settings' > 'Supported Templates'.
* But there's a flag for the WP-CLI command that sets this query var to validate all URLs.
*/
if ( AMP_Validation_Manager::is_theme_support_forced() ) {
return true;
}

$queried_object = get_queried_object();
if ( ! $is_legacy ) {
// Abort if in Transitional mode and AMP is not available for the URL.
Expand Down
16 changes: 16 additions & 0 deletions includes/class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1777,6 +1777,9 @@ public static function prepare_response( $response, $args = [] ) {
if ( is_bool( $status_code ) ) {
$status_code = 200; // Not a web server environment.
}
if ( ! headers_sent() ) {
header( 'Content-Type: application/json; charset=utf-8' );
}
return wp_json_encode(
[
'status_code' => $status_code,
Expand Down Expand Up @@ -1810,6 +1813,19 @@ public static function prepare_response( $response, $args = [] ) {
$response
)
) ) {
if ( AMP_Validation_Manager::$is_validate_request ) {
if ( ! headers_sent() ) {
status_header( 400 );
header( 'Content-Type: application/json; charset=utf-8' );
}
return wp_json_encode(
[
'code' => 'RENDERED_PAGE_NOT_AMP',
'message' => __( 'The requested URL did not result in an AMP page being rendered.', 'amp' ),
]
);
}

return $response;
}

Expand Down
70 changes: 0 additions & 70 deletions includes/embeds/class-amp-core-block-handler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ class AMP_Core_Block_Handler extends AMP_Base_Embed_Handler {
'core/categories' => 'ampify_categories_block',
'core/archives' => 'ampify_archives_block',
'core/video' => 'ampify_video_block',
'core/cover' => 'ampify_cover_block',
];

/**
Expand Down Expand Up @@ -212,75 +211,6 @@ public function ampify_video_block( $block_content, $block ) {
return $block_content;
}

/**
* Ampify cover block.
*
* This ensures that the background img/video in a cover block has object-fit=cover and the appropriate object-position
* attribute so that they will be carried over to to the amp-img/amp-video and propagated to the img/video in the
* light shadow DOM.
*
* @see \AMP_Video_Sanitizer::filter_video_dimensions()
*
* @param string $block_content The block content about to be appended.
* @param array $block The full block, including name and attributes.
* @return string Filtered block content.
*/
public function ampify_cover_block( $block_content, $block ) {
$is_video_background = (
isset( $block['attrs']['backgroundType'] )
&&
'video' === $block['attrs']['backgroundType']
);
$is_image_element = ! (
! empty( $block['attrs']['hasParallax'] )
||
! empty( $block['attrs']['isRepeated'] )
);

// Object fit/position is not relevant when background image has fixed positioning or is repeated.
// In other words, it is not relevant when a <video> or a <img> is not going to be used.
// See <https://github.com/WordPress/gutenberg/blob/54c9066d4/packages/block-library/src/cover/save.js#L54-L72>.
if ( ! ( $is_video_background || $is_image_element ) ) {
return $block_content;
}

$pattern = sprintf(
'#<%s(?= )[^>]*? class="(?:[^"]*? )?wp-block-cover__%s-background(?: [^"]*?)?"#',
$is_video_background ? 'video' : 'img',
$is_video_background ? 'video' : 'image'
);
return preg_replace_callback(
$pattern,
static function ( $matches ) use ( $block ) {
$replacement = $matches[0];

// The background image/video for the cover block by definition needs object-fit="cover" on the resulting amp-ing/amp-video.
$replacement .= ' object-fit="cover"';

// Add the fill layout to skip needlessly obtaining the dimensions.
$replacement .= ' layout="fill"';

// Add object-position from the block's attributes to add to the img/video to be copied onto the amp-img/amp-video.
// The AMP runtime copies object-position attribute onto the underlying img/video for a given amp-img/amp-video.
// This is needed since the object-position property directly on an amp-img/amp-video will have no effect since
// since it is merely a wrapper for the underlying img/video element which actually supports the CSS property.
if ( isset( $block['attrs']['focalPoint']['x'], $block['attrs']['focalPoint']['y'] ) ) {
// See logic in Gutenberg for writing focal point to object-position attr:
// <https://github.com/WordPress/gutenberg/blob/54c9066/packages/block-library/src/cover/save.js#L71>.
$replacement .= sprintf(
' object-position="%d%% %d%%"',
round( (float) $block['attrs']['focalPoint']['x'] * 100 ),
round( (float) $block['attrs']['focalPoint']['y'] * 100 )
);
}

return $replacement;
},
$block_content,
1
);
}

/**
* Sanitize widgets that are not added via Gutenberg.
*
Expand Down
114 changes: 6 additions & 108 deletions includes/sanitizers/class-amp-core-theme-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,18 +158,17 @@ protected static function get_theme_features_config( $theme_slug ) {
// Twenty Nineteen.
case 'twentynineteen':
return [
'dequeue_scripts' => [
'dequeue_scripts' => [
'twentynineteen-skip-link-focus-fix', // This is part of AMP. See <https://github.com/ampproject/amphtml/issues/18671>.
'twentynineteen-priority-menu',
'twentynineteen-touch-navigation', // @todo There could be an AMP implementation of this, similar to what is implemented on ampproject.org.
],
'remove_actions' => [
'remove_actions' => [
'wp_print_footer_scripts' => [
'twentynineteen_skip_link_focus_fix', // See <https://github.com/WordPress/twentynineteen/pull/47>.
],
],
'add_twentynineteen_masthead_styles' => [],
'adjust_twentynineteen_images' => [],
'adjust_twentynineteen_images' => [],
'enable_determine_hero_images_transformer' => [],
];

Expand Down Expand Up @@ -938,121 +937,20 @@ static function() use ( $method ) {
}

/**
* Add required styles for featured image header in Twenty Nineteen.
*
* The following is necessary because the styles in the theme apply to the featured img,
* and the CSS parser will then convert the selectors to amp-img. Nevertheless, object-fit
* does not apply on amp-img and it needs to apply on an actual img.
*
* @link https://github.com/WordPress/wordpress-develop/blob/5.0/src/wp-content/themes/twentynineteen/style.css#L2276-L2299
* @since 1.0
*/
public static function add_twentynineteen_masthead_styles() {
add_action(
'wp_enqueue_scripts',
static function() {
ob_start();
?>
<style>
.site-header.featured-image .site-featured-image .post-thumbnail amp-img > img {
height: auto;
left: 50%;
max-width: 1000%;
min-height: 100%;
min-width: 100vw;
position: absolute;
top: 50%;
transform: translateX(-50%) translateY(-50%);
width: auto;
z-index: 1;
/* When image filters are active, make it grayscale to colorize it blue. */
}

@supports (object-fit: cover) {
.site-header.featured-image .site-featured-image .post-thumbnail amp-img > img {
height: 100%;
left: 0;
object-fit: cover;
top: 0;
transform: none;
width: 100%;
}
}
</style>
<?php
$styles = str_replace( [ '<style>', '</style>' ], '', ob_get_clean() );
wp_add_inline_style( get_template() . '-style', $styles );
},
11
);
}

/**
* Add required styles for video and image headers.
* Add required styles for Twenty Seventeen header.
*
* This is currently used exclusively for Twenty Seventeen.
* This is required since JS is not applying the required styles at runtime.
*
* @since 1.0
* @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1687
* @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1743
*/
public static function add_twentyseventeen_masthead_styles() {
/*
* The following is necessary because the styles in the theme apply to img and video,
* and the CSS parser will then convert the selectors to amp-img and amp-video respectively.
* Nevertheless, object-fit does not apply on amp-img and it needs to apply on an actual img.
*/
add_action(
'wp_enqueue_scripts',
static function() {
$is_front_page_layout = ( is_front_page() && 'posts' !== get_option( 'show_on_front' ) ) || ( is_home() && is_front_page() );
ob_start();
?>
<style>
.has-header-image .custom-header-media amp-img > img,
.has-header-video .custom-header-media amp-video > video{
position: fixed;
height: auto;
left: 50%;
max-width: 1000%;
min-height: 100%;
min-width: 100%;
min-width: 100vw; /* vw prevents 1px gap on left that 100% has */
width: auto;
top: 50%;
padding-bottom: 1px; /* Prevent header from extending beyond the footer */
-ms-transform: translateX(-50%) translateY(-50%);
-moz-transform: translateX(-50%) translateY(-50%);
-webkit-transform: translateX(-50%) translateY(-50%);
transform: translateX(-50%) translateY(-50%);
}
.has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img {
bottom: 0;
position: absolute;
top: auto;
-ms-transform: translateX(-50%) translateY(0);
-moz-transform: translateX(-50%) translateY(0);
-webkit-transform: translateX(-50%) translateY(0);
transform: translateX(-50%) translateY(0);
}
/* For browsers that support object-fit */
@supports ( object-fit: cover ) {
.has-header-image .custom-header-media amp-img > img,
.has-header-video .custom-header-media amp-video > video,
.has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img {
height: 100%;
left: 0;
-o-object-fit: cover;
object-fit: cover;
top: 0;
-ms-transform: none;
-moz-transform: none;
-webkit-transform: none;
transform: none;
width: 100%;
}
}

.navigation-top.site-navigation-fixed {
display: none;
}
Expand Down Expand Up @@ -1555,7 +1453,7 @@ static function() {
font-size: 32px;
line-height: 46px;
}
.featured-content .post-thumbnail amp-img > img {
.featured-content .post-thumbnail amp-img {
object-fit: cover;
object-position: top;
}
Expand Down
15 changes: 11 additions & 4 deletions includes/sanitizers/trait-amp-noscript-fallback.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

use AmpProject\Dom\Document;
use AmpProject\Attribute;

/**
* Trait AMP_Noscript_Fallback
Expand Down Expand Up @@ -52,6 +53,13 @@ protected function initialize_noscript_allowed_attributes( $tag ) {
}
}
}

// Remove attributes which are likely to cause styling conflicts, as the noscript fallback should get treated like it has fill layout.
unset(
$this->noscript_fallback_allowed_attributes[ Attribute::ID ],
$this->noscript_fallback_allowed_attributes[ Attribute::CLASS_ ],
$this->noscript_fallback_allowed_attributes[ Attribute::STYLE ]
);
}

/**
Expand Down Expand Up @@ -81,13 +89,12 @@ protected function append_old_node_noscript( DOMElement $new_element, DOMElement
$noscript->appendChild( $old_element );
$new_element->appendChild( $noscript );

// Remove all non-allowed attributes preemptively to prevent doubled validation errors.
// Remove all non-allowed attributes preemptively to prevent doubled validation errors, only leaving the attributes required.
for ( $i = $old_element->attributes->length - 1; $i >= 0; $i-- ) {
$attribute = $old_element->attributes->item( $i );
if ( isset( $this->noscript_fallback_allowed_attributes[ $attribute->nodeName ] ) ) {
continue;
if ( ! isset( $this->noscript_fallback_allowed_attributes[ $attribute->nodeName ] ) ) {
$old_element->removeAttribute( $attribute->nodeName );
}
$old_element->removeAttribute( $attribute->nodeName );
}
}
}
Loading

0 comments on commit 80cf883

Please sign in to comment.