diff --git a/assets/src/components/higher-order/with-amp-story-settings.js b/assets/src/components/higher-order/with-amp-story-settings.js index 506afe0d67c..e1ff83c4225 100644 --- a/assets/src/components/higher-order/with-amp-story-settings.js +++ b/assets/src/components/higher-order/with-amp-story-settings.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { get } from 'lodash'; + /** * WordPress dependencies */ @@ -40,6 +45,7 @@ const applyFallbackStyles = withFallbackStyles( ( node, ownProps ) => { const applyWithSelect = withSelect( ( select, props ) => { const { getSelectedBlockClientId, getBlockRootClientId, getBlock } = select( 'core/block-editor' ); const { getAnimatedBlocks, isValidAnimationPredecessor } = select( 'amp/story' ); + const { getMedia } = select( 'core' ); const currentBlock = getSelectedBlockClientId(); const page = getBlockRootClientId( currentBlock ); @@ -47,6 +53,16 @@ const applyWithSelect = withSelect( ( select, props ) => { const animatedBlocks = getAnimatedBlocks()[ page ] || []; const animationOrderEntry = animatedBlocks.find( ( { id } ) => id === props.clientId ); + const isVideoBlock = 'core/video' === props.name; + let videoFeaturedImage; + + // If we have a video set from an attachment but there is no poster, use the featured image of the video if available. + if ( isVideoBlock && props.attributes.id && ! props.attributes.poster ) { + const media = getMedia( props.attributes.id ); + const featuredImage = media && get( media, [ '_links', 'wp:featuredmedia', 0, 'href' ], null ); + videoFeaturedImage = featuredImage && getMedia( Number( featuredImage.split( '/' ).pop() ) ); + } + return { parentBlock: getBlock( getBlockRootClientId( props.clientId ) ), // Use parent's clientId instead of anchor attribute. @@ -70,6 +86,7 @@ const applyWithSelect = withSelect( ( select, props ) => { }; } ); }, + videoFeaturedImage, }; } ); @@ -141,6 +158,7 @@ export default createHigherOrderComponent( onAnimationDelayChange, getAnimatedBlocks, animationAfter, + videoFeaturedImage, startBlockRotation, stopBlockRotation, } = props; @@ -152,6 +170,7 @@ export default createHigherOrderComponent( } const isImageBlock = 'core/image' === name; + const isVideoBlock = 'core/video' === name; const isTextBlock = 'amp/amp-story-text' === name; const needsTextSettings = BLOCKS_WITH_TEXT_SETTINGS.includes( name ); const isMovableBlock = ALLOWED_MOVABLE_BLOCKS.includes( name ); @@ -170,6 +189,11 @@ export default createHigherOrderComponent( rotationAngle, } = attributes; + // If we have a video set from an attachment but there is no poster, use the featured image of the video if available. + if ( isVideoBlock && videoFeaturedImage ) { + setAttributes( { poster: videoFeaturedImage.source_url } ); + } + const minTextHeight = 20; const minTextWidth = 30; diff --git a/assets/src/components/higher-order/with-video-poster-image-notice.js b/assets/src/components/higher-order/with-video-poster-image-notice.js new file mode 100644 index 00000000000..eee619ee76f --- /dev/null +++ b/assets/src/components/higher-order/with-video-poster-image-notice.js @@ -0,0 +1,57 @@ + +/** + * WordPress dependencies + */ +import { __ } from '@wordpress/i18n'; +import { select } from '@wordpress/data'; +import { Fragment } from '@wordpress/element'; +import { Notice } from '@wordpress/components'; + +/** + * Internal dependencies + */ +import { createHigherOrderComponent } from '@wordpress/compose'; + +/** + * Higher-order component that is used for adding an error notice for the poster image control for video blocks in AMP stories. + * + * Note that if a user has selected a poster image and then removed it, the poster attribute will remain but be empty. + * Because of this, an AMP validation error will not not always ensue if no poster image is supplied, as the plugin's sanitizer + * will supply an empty poster attribute to try to prevent a validation error from happening. The "amp-story >> amp-video" spec + * has a loose definition for amp-video[poster] in that only specifies that it is mandatory, but it leaves its value as being undefined. + * So this is why an empty poster attribute actually circumvents a validation error from happening. In the future if the value_url type + * is specified, then the sanitizer will not be able to supply an empty value to circumvent the AMP validation error. + * + * @link https://github.com/ampproject/amphtml/blob/b814e5d74cadf554c5caa1233d71e8e840788ff5/extensions/amp-video/validator-amp-video.protoascii#L147-L150 + * + * @return {Function} Higher-order component. + */ +export default createHigherOrderComponent( + ( MediaUpload ) => { + return ( props ) => { + const isAmpStory = 'amp_story' === select( 'core/editor' ).getCurrentPostType(); + const selectedBlock = select( 'core/block-editor' ).getSelectedBlock(); + + // The `! selectedBlock.attributes.src` check ensures that the notice is not added to the block placeholder. + // Otherwise, if a video has been selected, then it is presumed that the MediaUpload here is for the poster image. + // There is apparently no way to know for sure that this is for the poster, other than by checking if + // props.title === "Select Poster Image", but this would be quite brittle. + if ( ! isAmpStory || ! selectedBlock || 'core/video' !== selectedBlock.name || ! selectedBlock.attributes.src || selectedBlock.attributes.poster ) { + return ; + } + + return ( + + + { __( 'A poster is required for videos in stories.', 'amp' ) } + + + + ); + }; + }, + 'withVideoPosterImageNotice' +); diff --git a/assets/src/components/index.js b/assets/src/components/index.js index 09c31787e61..e7dc42893cc 100644 --- a/assets/src/components/index.js +++ b/assets/src/components/index.js @@ -32,3 +32,4 @@ export { default as withCroppedFeaturedImage } from './with-cropped-featured-ima export { default as withActivePageState } from './with-active-page-state'; export { default as withStoryBlockDropZone } from './with-story-block-drop-zone'; export { default as withCallToActionValidation } from './higher-order/with-call-to-action-validation'; +export { default as withVideoPosterImageNotice } from './higher-order/with-video-poster-image-notice'; diff --git a/assets/src/stories-editor/helpers/index.js b/assets/src/stories-editor/helpers/index.js index 4f34fdcbd6d..3a978fc0637 100644 --- a/assets/src/stories-editor/helpers/index.js +++ b/assets/src/stories-editor/helpers/index.js @@ -174,6 +174,7 @@ export const addAMPAttributes = ( settings, name ) => { } const isImageBlock = 'core/image' === name; + const isVideoBlock = 'core/video' === name; const isMovableBlock = ALLOWED_MOVABLE_BLOCKS.includes( name ); const addedAttributes = { @@ -256,6 +257,28 @@ export const addAMPAttributes = ( settings, name ) => { }; } + if ( isVideoBlock ) { + // Required defaults for AMP validity. + addedAttributes.autoplay = { + ...settings.attributes.autoplay, + default: true, + }; + addedAttributes.playsInline = { + ...settings.attributes.playsInline, + default: false, + }; + + // Optional defaults. + addedAttributes.loop = { + ...settings.attributes.loop, + default: true, + }; + addedAttributes.controls = { + ...settings.attributes.controls, + default: false, + }; + } + // Define selector according to mappings. if ( BLOCK_TAG_MAPPING[ name ] ) { addedAttributes.ampAnimationType = { diff --git a/assets/src/stories-editor/index.js b/assets/src/stories-editor/index.js index 775a95b953f..86fa93e98b2 100644 --- a/assets/src/stories-editor/index.js +++ b/assets/src/stories-editor/index.js @@ -29,6 +29,7 @@ import { withActivePageState, withStoryBlockDropZone, withCallToActionValidation, + withVideoPosterImageNotice, } from '../components'; import { maybeEnqueueFontStyle, @@ -277,6 +278,7 @@ addFilter( 'editor.PostFeaturedImage', 'ampStoryEditorBlocks/addFeaturedImageNot addFilter( 'editor.BlockListBlock', 'ampStoryEditorBlocks/withActivePageState', withActivePageState ); addFilter( 'editor.BlockListBlock', 'ampStoryEditorBlocks/addWrapperProps', withWrapperProps ); addFilter( 'editor.MediaUpload', 'ampStoryEditorBlocks/addCroppedFeaturedImage', ( InitialMediaUpload ) => withCroppedFeaturedImage( InitialMediaUpload, getMinimumStoryPosterDimensions() ) ); +addFilter( 'editor.MediaUpload', 'ampStoryEditorBlocks/addPosterImageNotice', withVideoPosterImageNotice ); addFilter( 'blocks.getSaveContent.extraProps', 'ampStoryEditorBlocks/addExtraAttributes', addAMPExtraProps ); addFilter( 'blocks.getSaveElement', 'ampStoryEditorBlocks/wrapBlocksInGridLayer', wrapBlocksInGridLayer ); addFilter( 'editor.BlockDropZone', 'ampStoryEditorBlocks/withStoryBlockDropZone', withStoryBlockDropZone ); diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index cd65b1a5f75..5636ab87a09 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -393,14 +393,17 @@ def GetTagSpec(tag_spec, attr_lists): tag_dict = GetTagRules(tag_spec) if tag_dict is None: return None - attr_dict = GetAttrs(tag_spec.attrs) + attr_dict = {} - # Now add attributes from any attribute lists to this tag. + # First add attributes from any attribute lists to this tag. for (tag_field_desc, tag_field_val) in tag_spec.ListFields(): if 'attr_lists' == tag_field_desc.name: for attr_list in tag_field_val: attr_dict.update(attr_lists[UnicodeEscape(attr_list)]) + # Then merge the spec-specific attributes on top to override any list definitions. + attr_dict.update(GetAttrs(tag_spec.attrs)) + logging.info('... done') tag_spec_dict = {'tag_spec':tag_dict, 'attr_spec_list':attr_dict} if tag_spec.HasField('cdata'): diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php index 5ccbd8e5e77..26751f8e64d 100644 --- a/includes/sanitizers/class-amp-allowed-tags-generated.php +++ b/includes/sanitizers/class-amp-allowed-tags-generated.php @@ -5306,6 +5306,7 @@ class AMP_Allowed_Tags_Generated { 'artwork' => array(), 'attribution' => array(), 'autoplay' => array( + 'mandatory' => true, 'value' => array( '', ), @@ -16112,7 +16113,6 @@ class AMP_Allowed_Tags_Generated { '', ), ), - 'grid-area' => array(), 'hidden' => array( 'value' => array( '',