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

Add AMP validation checking for Gutenberg blocks #1019

Merged
merged 49 commits into from
Apr 16, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
8413457
Add source comments around each Gutenberg block to track validation i…
westonruter Mar 13, 2018
07aeee2
Defer mustache tag replacements to right before serialization and onl…
westonruter Mar 14, 2018
01691e5
Eliminate needless use of PEG parser for adding block source comments
westonruter Mar 17, 2018
27e8bec
Revert commit that removed REST API logic.
Mar 18, 2018
590cd76
Prototype asynchronous notices for blocks.
Mar 18, 2018
8218919
Rever commit 27e8b, which added REST API endpoint.
Mar 19, 2018
9c3568d
Add amp_validation field to REST API response.
Mar 19, 2018
ab82e8d
Output validation errors in REST API response.
Mar 19, 2018
ea1c37a
Remove jQuery dependency and ES6 class.
Mar 20, 2018
96297fc
Change which post types have the added field.
Mar 20, 2018
ed4b03b
In REST API response, validate front-end if no errors exist.
Mar 21, 2018
561af32
Skip Gutenberg-based tests for WP version < 4.9.
Mar 21, 2018
f64ab2e
Begin to add notices to blocks based on errors.
Mar 21, 2018
b9bd206
Address Travis error by aligning array values vertically.
Mar 21, 2018
bcdff5f
Get the block types with errors from the REST API response.
Mar 22, 2018
049e482
Update test to reflect change in text.
Mar 22, 2018
fe34e8e
Store the block validation errors, avoiding lookup in every edit().
Mar 22, 2018
49b66ae
Get validation errors for specific blocks, not only for block names.
Mar 22, 2018
743cca4
Address Travis errors by raising variable declaration.
Mar 22, 2018
4676777
Add 'block_attrs' to blocksWithErrors.
Mar 22, 2018
b9aa16c
Correct the variable for block_attrs.
Mar 22, 2018
e2fc9a7
Use the new blockAttrs to find a match with errors.
Mar 23, 2018
6cd996a
Move the notice from below to above the block.
Mar 23, 2018
339f69f
Add a 'More details' link to the notice.
Mar 23, 2018
a02d029
Enable showing multiple validation errors.
Mar 23, 2018
15f0abe
Enable outputting several error codes, and their counts.
Mar 23, 2018
2b03c6a
Address a Travis error regarding complexity.
Mar 23, 2018
d366da3
Remove the counts from after the error codes.
Mar 23, 2018
80d32de
Add keys to the components and edit block.
Mar 23, 2018
16388c2
Make the notice expandable.
Mar 26, 2018
f9d8575
Merge in develop, resolve conflicts.
kienstra Apr 6, 2018
9aa6704
Address Travis error by removing extra comma.
kienstra Apr 6, 2018
dcb1e77
Force re-validation of post on frontend for amp_validation_errors fie…
westonruter Apr 8, 2018
eb00bcf
Prevent re-validating posts that have just been validated
westonruter Apr 8, 2018
9ecc24c
Fix: Each child in an array or iterator should have a unique "key" prop
westonruter Apr 8, 2018
354fdcd
Remove block error summary while waiting for design to formulate
westonruter Apr 8, 2018
0bada73
Fix delay between save and update of validation error notice
westonruter Apr 9, 2018
34024cc
Use eslint-config-wordpress and fix eslint issues
westonruter Apr 9, 2018
8e78904
Prevent reporting validation errors for blocks that are not in the cu…
westonruter Apr 9, 2018
277da63
Use eslint config adapted from Gutenberg
westonruter Apr 10, 2018
ca3ff60
Use block content index to match blocks with corresponding validation…
westonruter Apr 10, 2018
01a21a8
Only update block validation errors when editor state is clean
westonruter Apr 10, 2018
3e032d4
Handle showing validation errors for nested blocks
westonruter Apr 11, 2018
6a4cbcc
Add initial overall warning notice when there are validation errors
westonruter Apr 11, 2018
4d7dc19
Improve organization of Gutenberg extension code; improve warning not…
westonruter Apr 11, 2018
21e60f0
Show details with each block's validation errors; improve styling
westonruter Apr 11, 2018
ca6c96c
Add link to validation error details in Gutenberg notice
westonruter Apr 12, 2018
7a60509
Use wp.element.Fragment instead of wrangling arrays with key props
westonruter Apr 12, 2018
5bf12da
Merge branch 'develop' of https://github.com/Automattic/amp-wp into a…
westonruter Apr 13, 2018
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
1 change: 1 addition & 0 deletions .dev-lib
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ PROJECT_SLUG=amp
function after_wp_install {
echo "Installing REST API..."
svn export -q https://plugins.svn.wordpress.org/jetpack/trunk/ "$WP_CORE_DIR/src/wp-content/plugins/jetpack"
svn export -q https://plugins.svn.wordpress.org/gutenberg/trunk/ "$WP_CORE_DIR/src/wp-content/plugins/gutenberg"
}
185 changes: 185 additions & 0 deletions assets/js/amp-block-validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
/*jshint esversion: 6 */
/*global _, wp:true */
/**
* AMP Gutenberg integration.
*
* On editing a block, this checks that the content is AMP-compatible.
* And it displays a notice if it's not.
*/

/* exported ampBlockValidation */
var ampBlockValidation = ( function( $ ) {
var module = {

/**
* Holds data.
*/
data: {},

/**
* Boot module.
*
* @param {Object} data Object data.
* @return {void}
*/
boot: function( data ) {
module.data = data;
$( document ).ready( function() {
if ( 'undefined' !== typeof wp.blocks ) {
module.processBlocks();
}
} );
},

/**
* Gets all of the registered blocks, and overwrites their edit() functions.
*
* The new edit() functions will check if the content is AMP-compliant.
* If not, it will display a notice.
*
* @returns {void}
*/
processBlocks: function() {
var blocks = wp.blocks.getBlockTypes();
blocks.forEach( function( block ) {
if ( block.hasOwnProperty( 'name' ) ) {
module.overwriteEdit( block.name );
}
} );
},

/**
* Overwrites the edit() function of a block.
*
* Outputs the original edit function, stored in OriginalBlockEdit.
* This also appends a notice to the block.
* It only displays if the block's content isn't valid AMP,
*
* @see https://riad.blog/2017/10/16/one-thousand-and-one-way-to-extend-gutenberg-today/
* @param {string} blockType the type of the block, like 'core/paragraph'.
* @returns {void}
*/
overwriteEdit: function( blockType ) {
let block = wp.blocks.unregisterBlockType( blockType );
let OriginalBlockEdit = block.edit;

block.edit = class AMPNotice extends wp.element.Component {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES6 classes aren't going to work in IE11.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @westonruter. This commit removes the ES6 class.


/**
* The AMPNotice constructor.
*
* @param {object} props The component properties.
* @returns {void}
*/
constructor( props ) {
props.attributes.pendingValidation = false;
super( props );
this.validateAMP = _.throttle( this.validateAMP, 5000 );
this.state = { isInvalidAMP: false };
}

/**
* Outputs the existing block, with a Notice element below.
*
* The Notice only appears if the state of isInvalidAMP is false.
* It also displays the name of the block.
*
* @returns {array} The elements.
*/
render() {
let originalEdit;
let result;

result = [];
originalEdit = wp.element.createElement( OriginalBlockEdit, this.props );
if ( originalEdit ) {
result.push( originalEdit );
}
if ( this.state.isInvalidAMP && wp.components.hasOwnProperty( 'Notice' ) ) {
result.push( wp.element.createElement(
wp.components.Notice,
{
status: 'warning',
content: module.data.i18n.notice.replace( '%s', this.props.name ),
isDismissible: false
}
) );
}

this.props.attributes.pendingValidation = false;
return result;
}

/**
* Handler for after the component mounting.
*
* If validateAMP() changes the isInvalidAMP state, it will result in this method being called again.
* There's no need to check if the state is valid again.
* So this skips the check if pendingValidation is true.
*
* @returns {void}
*/
componentDidMount() {
if ( ! this.props.attributes.pendingValidation ) {
let content = this.props.attributes.content;
if ( 'string' !== typeof content ) {
content = wp.element.renderToString( content );
}
if ( content.length > 0 ) {
this.validateAMP( this.props.attributes.content );
}
}
this.props.attributes.pendingValidation = false;
}

/**
* Validates the content for AMP compliance, and sets the state of the Notice.
*
* Depending on the results of the validation,
* it sets the Notice component's isInvalidAMP state.
* This will cause the notice to either display or be hidden.
*
* @param {string} content The block content, from calling save().
* @returns {void}
*/
validateAMP( content ) {
this.setState( function() {

// Changing the state can cause componentDidMount() to be called, so prevent it from calling validateAMP() again.
component.props.attributes.pendingValidation = true;
return { isInvalidAMP: ( Math.random() > 0.5 ) };
} );

let component = this;
$.post(
module.data.endpoint,
{
markup: content
}
).done( function( data ) {
if ( data.hasOwnProperty( 'removed_elements' ) && ( 0 === data.removed_elements.length ) && ( 0 === data.removed_attributes.length ) ) {
component.setState( function() {

// Changing the state can cause componentDidMount() to be called, so prevent it from calling validateAMP() again.
component.props.attributes.pendingValidation = true;
return { isInvalidAMP: false };
} );
} else {
component.setState( function() {
component.props.attributes.pendingValidation = true;
return { isInvalidAMP: true };
} );
}
} );
}

};

wp.blocks.registerBlockType( blockType, block );
}

};

return module;

} )( window.jQuery );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should eliminate jQuery as being a dependency for this script. Extending Gutenberg should be jQuery-free.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @westonruter. This commit removes the jQuery dependency.

2 changes: 1 addition & 1 deletion includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ public static function prepare_response( $response, $args = array() ) {
return $response;
}

$is_validation_debug_mode = ! empty( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ); // WPCS: csrf ok.
$is_validation_debug_mode = isset( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ); // WPCS: csrf ok.

$args = array_merge(
array(
Expand Down
62 changes: 38 additions & 24 deletions includes/utils/class-amp-dom-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,6 @@ public static function get_dom( $document ) {
// @todo In the future consider an AMP_DOMDocument subclass that does this automatically. See <https://github.com/Automattic/amp-wp/pull/895/files#r163825513>.
$document = self::convert_amp_bind_attributes( $document );

/*
* Prevent amp-mustache syntax from getting URL-encoded in attributes when saveHTML is done.
* While this is applying to the entire document, it only really matters inside of <template>
* elements, since URL-encoding of curly braces in href attributes would not normally matter.
* But when this is done inside of a <template> then it breaks Mustache. Since Mustache
* is logic-less and curly braces are not unsafe for HTML, we can do a global replacement.
* The replacement is done on the entire HTML document instead of just inside of the <template>
* elements since it is faster and wouldn't change the outcome.
*/
$placeholders = self::get_mustache_tag_placeholders();
$document = str_replace(
array_keys( $placeholders ),
array_values( $placeholders ),
$document
);

// Force all self-closing tags to have closing tags since DOMDocument isn't fully aware.
$document = preg_replace(
'#<(' . implode( '|', self::$self_closing_tags ) . ')[^>]*>(?!</\1>)#',
Expand Down Expand Up @@ -365,13 +349,51 @@ public static function get_content_from_dom_node( $dom, $node ) {
$self_closing_tags_regex = "#</({$self_closing_tags})>#i";
}

/*
* Prevent amp-mustache syntax from getting URL-encoded in attributes when saveHTML is done.
* While this is applying to the entire document, it only really matters inside of <template>
* elements, since URL-encoding of curly braces in href attributes would not normally matter.
* But when this is done inside of a <template> then it breaks Mustache. Since Mustache
* is logic-less and curly braces are not unsafe for HTML, we can do a global replacement.
* The replacement is done on the entire HTML document instead of just inside of the <template>
* elements since it is faster and wouldn't change the outcome.
*/
$mustache_tag_placeholders = self::get_mustache_tag_placeholders();
$mustache_tags_replaced = false;
$xpath = new DOMXPath( $dom );
$templates = $dom->getElementsByTagName( 'template' );
foreach ( $templates as $template ) {

// These attributes are the only ones that saveHTML() will URL-encode.
foreach ( $xpath->query( './/*/@src|.//*/@href|.//*/@action', $template ) as $attribute ) {
$attribute->nodeValue = str_replace(
array_keys( $mustache_tag_placeholders ),
array_values( $mustache_tag_placeholders ),
$attribute->nodeValue,
$count
);
if ( $count ) {
$mustache_tags_replaced = true;
}
}
}

$html = $dom->saveHTML( $node );

// Whitespace just causes unit tests to fail... so whitespace begone.
if ( '' === trim( $html ) ) {
return '';
}

// Restore amp-mustache placeholders which were replaced to prevent URL-encoded corruption by saveHTML.
if ( $mustache_tags_replaced ) {
$html = str_replace(
array_values( $mustache_tag_placeholders ),
array_keys( $mustache_tag_placeholders ),
$html
);
}

// Restore noscript elements which were temporarily removed to prevent libxml<2.8 parsing problems.
if ( version_compare( LIBXML_DOTTED_VERSION, '2.8', '<' ) ) {
$html = str_replace(
Expand All @@ -383,14 +405,6 @@ public static function get_content_from_dom_node( $dom, $node ) {

$html = self::restore_amp_bind_attributes( $html );

// Restore amp-mustache placeholders which were replaced to prevent URL-encoded corruption by saveHTML.
$placeholders = self::get_mustache_tag_placeholders();
$html = str_replace(
array_values( $placeholders ),
array_keys( $placeholders ),
$html
);

/*
* Travis w/PHP 7.1 generates <br></br> and <hr></hr> vs. <br/> and <hr/>, respectively.
* Travis w/PHP 7.x generates <source ...></source> vs. <source ... />. Etc.
Expand Down
Loading