Skip to content

Commit

Permalink
Merge pull request #1861 from ampproject/add/img-noscript-fallback
Browse files Browse the repository at this point in the history 
Add noscript fallbacks for img, audio, and video; improve audio conversion to amp-audio; add AMP [fallback] for amp-audio/amp-video
  • Loading branch information
@westonruter
westonruter authored Feb 18, 2019
2 parents 0f74bdb + 68cea88 commit bc59039
Show file tree
Hide file tree
Showing 10 changed files with 481 additions and 188 deletions.
7 changes: 1 addition & 6 deletions assets/js/amp-editor-blocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
value: 'responsive',
label: __( 'Responsive', 'amp' ),
notAvailable: [
'core/audio',
'core-embed/soundcloud'
]
},
Expand All @@ -50,15 +49,13 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
value: 'fill',
label: __( 'Fill', 'amp' ),
notAvailable: [
'core/audio',
'core-embed/soundcloud'
]
},
{
value: 'flex-item',
label: __( 'Flex Item', 'amp' ),
notAvailable: [
'core/audio',
'core-embed/soundcloud'
]
},
Expand All @@ -67,7 +64,6 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
value: 'intrinsic',
label: __( 'Intrinsic', 'amp' ),
notAvailable: [
'core/audio',
'core-embed/youtube',
'core-embed/facebook',
'core-embed/instagram',
Expand All @@ -83,8 +79,7 @@ var ampEditorBlocks = ( function() { // eslint-disable-line no-unused-vars
defaultHeight: 400,
mediaBlocks: [
'core/image',
'core/video',
'core/audio'
'core/video'
],
textBlocks: [
'core/paragraph',
Expand Down
2 changes: 1 addition & 1 deletion includes/class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ function() {
add_filter(
'get_custom_logo',
function( $html ) {
return preg_replace( '/(?<=<img\s)/', ' noloading ', $html );
return preg_replace( '/(?<=<img\s)/', ' data-amp-noloading="" ', $html );
},
1
);
Expand Down
118 changes: 76 additions & 42 deletions includes/sanitizers/class-amp-audio-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,67 +44,101 @@ public function sanitize() {
}

for ( $i = $num_nodes - 1; $i >= 0; $i-- ) {
$node = $nodes->item( $i );
$amp_data = $this->get_data_amp_attributes( $node );
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );
$old_attributes = $this->filter_data_amp_attributes( $old_attributes, $amp_data );
$node = $nodes->item( $i );

$new_attributes = $this->filter_attributes( $old_attributes );
// Allow audio in fallbacks.
if ( 'noscript' === $node->parentNode->nodeName ) {
continue;
}

$new_node = AMP_DOM_Utils::create_node( $this->dom, 'amp-audio', $new_attributes );
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );

foreach ( $node->childNodes as $child_node ) {
// For amp-audio, the default width and height are inferred from browser.
$sources = array();
$new_attributes = $this->filter_attributes( $old_attributes );
if ( ! empty( $new_attributes['src'] ) ) {
$sources[] = $new_attributes['src'];
}

/**
* Child node.
*
* @todo: Fix when `source` has no closing tag as DOMDocument does not handle well.
*
* @var DOMNode $child_node
*/
/**
* Original node.
*
* @var DOMElement $old_node
*/
$old_node = $node->cloneNode( false );

// Gather all child nodes and supply empty video dimensions from sources.
$fallback = null;
$child_nodes = array();
while ( $node->firstChild ) {
$child_node = $node->removeChild( $node->firstChild );
if ( $child_node instanceof DOMElement && 'source' === $child_node->nodeName && $child_node->hasAttribute( 'src' ) ) {
$src = $this->maybe_enforce_https_src( $child_node->getAttribute( 'src' ), true );
if ( ! $src ) {
// @todo $this->remove_invalid_child( $child_node ), but this will require refactoring the while loop since it uses firstChild.
continue; // Skip adding source.
}
$sources[] = $src;
$child_node->setAttribute( 'src', $src );
$new_attributes = $this->filter_attributes( $new_attributes );
}

$new_child_node = $child_node->cloneNode( true );
if ( ! $new_child_node instanceof DOMElement ) {
continue;
if ( ! $fallback && $child_node instanceof DOMElement && ! ( 'source' === $child_node->nodeName || 'track' === $child_node->nodeName ) ) {
$fallback = $child_node;
$fallback->setAttribute( 'fallback', '' );
}

$old_child_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $new_child_node );
$new_child_attributes = $this->filter_attributes( $old_child_attributes );
$child_nodes[] = $child_node;
}

if ( empty( $new_child_attributes['src'] ) ) {
continue;
}
if ( 'source' !== $new_child_node->tagName ) {
continue;
}
/*
* Add fallback for audio shortcode which is not present by default since wp_mediaelement_fallback()
* is not called when wp_audio_shortcode_library is filtered from mediaelement to amp.
*/
if ( ! $fallback && ! empty( $sources ) ) {
$fallback = $this->dom->createElement( 'a' );
$fallback->setAttribute( 'href', $sources[0] );
$fallback->setAttribute( 'fallback', '' );
$fallback->appendChild( $this->dom->createTextNode( $sources[0] ) );
$child_nodes[] = $fallback;
}

// The textContent is invalid for `source` nodes.
$new_child_node->textContent = null;
/*
* Audio in WordPress is responsive with 100% width, so this infers fixed-layout.
* In AMP, the amp-audio's default height is inferred from the browser.
*/
$new_attributes['width'] = 'auto';

// Only append source tags with a valid src attribute.
$new_node->appendChild( $new_child_node );
// @todo Make sure poster and artwork attributes are HTTPS.
$new_node = AMP_DOM_Utils::create_node( $this->dom, 'amp-audio', $new_attributes );
foreach ( $child_nodes as $child_node ) {
$new_node->appendChild( $child_node );
if ( ! ( $child_node instanceof DOMElement ) || ! $child_node->hasAttribute( 'fallback' ) ) {
$old_node->appendChild( $child_node->cloneNode( true ) );
}
}

// Make sure the updated src and poster are applied to the original.
foreach ( array( 'src', 'poster', 'artwork' ) as $attr_name ) {
if ( $new_node->hasAttribute( $attr_name ) ) {
$old_node->setAttribute( $attr_name, $new_node->getAttribute( $attr_name ) );
}
}

/**
/*
* If the node has at least one valid source, replace the old node with it.
* Otherwise, just remove the node.
*
* @todo: Add a fallback handler.
* @see: https://github.com/ampproject/amphtml/issues/2261
* @todo Add a fallback handler.
* See: https://github.com/ampproject/amphtml/issues/2261
*/
if ( 0 === $new_node->childNodes->length && empty( $new_attributes['src'] ) ) {
if ( empty( $sources ) ) {
$this->remove_invalid_child( $node );
} else {

$layout = isset( $new_attributes['layout'] ) ? $new_attributes['layout'] : false;

// The width has to be unset / auto in case of fixed-height.
if ( 'fixed-height' === $layout ) {
$new_node->setAttribute( 'width', 'auto' );
}

$noscript = $this->dom->createElement( 'noscript' );
$new_node->appendChild( $noscript );
$node->parentNode->replaceChild( $new_node, $node );
$noscript->appendChild( $old_node );
}

$this->did_convert_elements = true;
Expand Down Expand Up @@ -163,7 +197,7 @@ private function filter_attributes( $attributes ) {
break;

default:
break;
$out[ $name ] = $value;
}
}

Expand Down
13 changes: 10 additions & 3 deletions includes/sanitizers/class-amp-img-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ private function adjust_and_replace_nodes_in_array_map( $node_lists ) {
/**
* Make final modifications to DOMNode
*
* @param DOMElement $node The DOMNode to adjust and replace.
* @param DOMElement $node The img element to adjust and replace.
*/
private function adjust_and_replace_node( $node ) {

Expand All @@ -254,9 +254,16 @@ private function adjust_and_replace_node( $node ) {
} else {
$new_tag = 'amp-img';
}
$new_node = AMP_DOM_Utils::create_node( $this->dom, $new_tag, $new_attributes );
$new_node = $this->handle_centering( $new_node );

$img_node = AMP_DOM_Utils::create_node( $this->dom, $new_tag, $new_attributes );
$new_node = $this->handle_centering( $img_node );
$node->parentNode->replaceChild( $new_node, $node );

// Preserve original node in noscript for no-JS environments.
$noscript = $this->dom->createElement( 'noscript' );
$noscript->appendChild( $node );
$img_node->appendChild( $noscript );

$this->add_auto_width_to_figure( $new_node );
}

Expand Down
5 changes: 5 additions & 0 deletions includes/sanitizers/class-amp-script-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public function sanitize() {
continue;
}

// Skip noscript elements inside of amp-img or other AMP components for fallbacks. See \AMP_Img_Sanitizer::adjust_and_replace_node().
if ( 'amp-' === substr( $noscript->parentNode->nodeName, 0, 4 ) ) {
continue;
}

$fragment = $this->dom->createDocumentFragment();
$fragment->appendChild( $this->dom->createComment( 'noscript' ) );
while ( $noscript->firstChild ) {
Expand Down
50 changes: 44 additions & 6 deletions includes/sanitizers/class-amp-video-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,34 @@ public function sanitize() {
}

for ( $i = $num_nodes - 1; $i >= 0; $i-- ) {
$node = $nodes->item( $i );
$node = $nodes->item( $i );

// Allow video in fallbacks.
if ( 'noscript' === $node->parentNode->nodeName ) {
continue;
}

$amp_data = $this->get_data_amp_attributes( $node );
$old_attributes = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $node );
$old_attributes = $this->filter_data_amp_attributes( $old_attributes, $amp_data );

$sources_count = 0;
$sources = array();
$new_attributes = $this->filter_attributes( $old_attributes );
$layout = isset( $amp_data['layout'] ) ? $amp_data['layout'] : false;
if ( isset( $new_attributes['src'] ) ) {
$new_attributes = $this->filter_video_dimensions( $new_attributes, $new_attributes['src'] );
if ( $new_attributes['src'] ) {
$sources_count++;
$sources[] = $new_attributes['src'];
}
}

/**
* Original node.
*
* @var DOMElement $old_node
*/
$old_node = $node->cloneNode( false );

// Gather all child nodes and supply empty video dimensions from sources.
$fallback = null;
$child_nodes = array();
Expand All @@ -83,7 +96,7 @@ public function sanitize() {
// @todo $this->remove_invalid_child( $child_node ), but this will require refactoring the while loop since it uses firstChild.
continue; // Skip adding source.
}
$sources_count++;
$sources[] = $src;
$child_node->setAttribute( 'src', $src );
$new_attributes = $this->filter_video_dimensions( $new_attributes, $src );
}
Expand All @@ -96,6 +109,18 @@ public function sanitize() {
$child_nodes[] = $child_node;
}

/*
* Add fallback for audio shortcode which is not present by default since wp_mediaelement_fallback()
* is not called when wp_audio_shortcode_library is filtered from mediaelement to amp.
*/
if ( ! $fallback && ! empty( $sources ) ) {
$fallback = $this->dom->createElement( 'a' );
$fallback->setAttribute( 'href', $sources[0] );
$fallback->setAttribute( 'fallback', '' );
$fallback->appendChild( $this->dom->createTextNode( $sources[0] ) );
$child_nodes[] = $fallback;
}

$new_attributes = $this->filter_attachment_layout_attributes( $node, $new_attributes, $layout );
if ( empty( $new_attributes['layout'] ) && ! empty( $new_attributes['width'] ) && ! empty( $new_attributes['height'] ) ) {
$new_attributes['layout'] = 'responsive';
Expand All @@ -106,19 +131,32 @@ public function sanitize() {
$new_node = AMP_DOM_Utils::create_node( $this->dom, 'amp-video', $new_attributes );
foreach ( $child_nodes as $child_node ) {
$new_node->appendChild( $child_node );
if ( ! ( $child_node instanceof DOMElement ) || ! $child_node->hasAttribute( 'fallback' ) ) {
$old_node->appendChild( $child_node->cloneNode( true ) );
}
}

// Make sure the updated src and poster are applied to the original.
foreach ( array( 'src', 'poster', 'artwork' ) as $attr_name ) {
if ( $new_node->hasAttribute( $attr_name ) ) {
$old_node->setAttribute( $attr_name, $new_node->getAttribute( $attr_name ) );
}
}

/*
* If the node has at least one valid source, replace the old node with it.
* Otherwise, just remove the node.
*
* TODO: Add a fallback handler.
* @todo Add a fallback handler.
* See: https://github.com/ampproject/amphtml/issues/2261
*/
if ( 0 === $sources_count ) {
if ( empty( $sources ) ) {
$this->remove_invalid_child( $node );
} else {
$noscript = $this->dom->createElement( 'noscript' );
$new_node->appendChild( $noscript );
$node->parentNode->replaceChild( $new_node, $node );
$noscript->appendChild( $old_node );
}

$this->did_convert_elements = true;
Expand Down
Loading

0 comments on commit bc59039

Please sign in to comment.