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.

Sign up for GitHub

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

Add noscript fallbacks for img, audio, and video; improve audio conversion to amp-audio; add AMP [fallback] for amp-audio/amp-video #1861

Merged
merged 13 commits into from
Feb 18, 2019
Merged

Add noscript fallbacks for img, audio, and video; improve audio conversion to amp-audio; add AMP [fallback] for amp-audio/amp-video #1861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
3186b0e
Append original img to amp-img under noscript for no-JS browsers
westonruter Feb 11, 2019
1bd3161
Allow img/video/audio inside of noscript
westonruter Feb 13, 2019
f1695cf
Add tests for noscript img fallbacks
westonruter Feb 13, 2019
ea48513
Fix test to account for noscript img fallback
westonruter Feb 13, 2019
e8fafa2
Add noscript audio fallbacks for amp-audio
westonruter Feb 13, 2019
66d029f
WIP: Add video noscript fallback support
westonruter Feb 13, 2019
1e4d697
Add data-amp-noloading to img for custom logo instead of just noloading
westonruter Feb 15, 2019
6b14816
Fix video sanitizer tests
westonruter Feb 15, 2019
7fc33bc
Remove amp-layout from audio since irrelevant
westonruter Feb 15, 2019
bad5193
Align audio sanitizer with video sanitizer
westonruter Feb 15, 2019
21c09d5
Fix eslint error for trailing comma
westonruter Feb 15, 2019
18af72f
Implement AMP equivalent to wp_mediaelement_fallback()
westonruter Feb 15, 2019
68cea88
Implement AMP video equivalent for wp_mediaelement_fallback()
westonruter Feb 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions assets/js/amp-editor-blocks.js
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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