Parser: Use an HTML like attribute syntax for comment attributes (#626)

* Parser: Use an HTML like attribute syntax for comment attributes * Parser: improve HTML attributes parsing * Parser: Test both parsing strategies
WordPress · May 5, 2017 · cbaecfb · cbaecfb
1 parent 3225c46
commit cbaecfb
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 131 deletions.
diff --git a/blocks/api/parser.js b/blocks/api/parser.js
@@ -99,7 +99,7 @@ export function parseWithTinyMCE( content ) {
 	//   In  : <!-- wp:core/embed url:youtube.com/xxx& -->
 	//   Out : <wp-block slug="core/embed" attributes="url:youtube.com/xxx&amp;">
 	content = content.replace(
-		/<!--\s*(\/?)wp:([a-z0-9/-]+)((?:\s+[a-z0-9_-]+:[^\s]+)*)\s*-->/g,
+		/<!--\s*(\/?)wp:([a-z0-9/-]+)((?:\s+[a-z0-9_-]+(="[^"]*")?)*)\s*-->/g,
 		function( match, closingSlash, slug, attributes ) {
 			if ( closingSlash ) {
 				return '</wp-block>';
@@ -166,15 +166,15 @@ export function parseWithTinyMCE( content ) {
 			}, {} );
 
 			// Retrieve the block attributes from the original delimiters
-			const blockAttributes = unescape( nodeAttributes.attributes || '' )
-				.split( /\s+/ )
-				.reduce( ( memo, attrString ) => {
-					const pieces = attrString.match( /^([a-z0-9_-]+):(.*)$/ );
-					if ( pieces ) {
-						memo[ pieces[ 1 ] ] = pieces[ 2 ];
-					}
-					return memo;
-				}, {} );
+			const attributesMatcher = /([a-z0-9_-]+)(="([^"]*)")?/g;
+			const blockAttributes = {};
+			let match;
+			do {
+				match = attributesMatcher.exec( unescape( nodeAttributes.attributes || '' ) );
+				if ( match ) {
+					blockAttributes[ match[ 1 ] ] = !! match[ 2 ] ? match[ 3 ] : true;
+				}
+			} while ( !! match );
 
 			// Try to create the block
 			const block = createBlockWithFallback(

diff --git a/blocks/api/post.pegjs b/blocks/api/post.pegjs
@@ -26,7 +26,7 @@ WP_Block_Html
   }
 
 WP_Block_Start
-  = "<!--" __ "wp:" blockType:WP_Block_Type attrs:WP_Block_Attribute_List _? "-->"
+  = "<!--" __ "wp:" blockType:WP_Block_Type attrs:HTML_Attribute_List _? "-->"
   { return {
     blockType: blockType,
     attrs: attrs
@@ -41,22 +41,38 @@ WP_Block_End
 WP_Block_Type
   = $(ASCII_Letter WP_Block_Type_Char*)
 
-WP_Block_Attribute_List
-  = as:(_+ attr:WP_Block_Attribute { return attr })*
-  { return as.reduce( function( attrs, pair ) {
-    attrs[ pair.name ] = pair.value;
-    return attrs;
-  }, {} ) }
-
-WP_Block_Attribute
-  = name:WP_Block_Attribute_Name ":" value:WP_Block_Attribute_Value
-  { return { name: name, value: value }; }
-
-WP_Block_Attribute_Name
-  = $(ASCII_Letter ASCII_AlphaNumeric*)
-
-WP_Block_Attribute_Value
-  = $(ASCII_Letter WP_Block_Attribute_Value_Char*)
+HTML_Attribute_List
+  = as:(_+ a:HTML_Attribute_Item { return a })*
+  { return as.reduce( function( attrs, currentAttribute ) {
+			var currentAttrs = {};
+			currentAttrs[ currentAttribute[ 0 ] ] = currentAttribute[ 1 ];
+			return Object.assign(
+				attrs,
+				currentAttrs
+			);
+	}, {} ) }
+
+HTML_Attribute_Item
+  = HTML_Attribute_Quoted
+  / HTML_Attribute_Unquoted
+  / HTML_Attribute_Empty
+
+HTML_Attribute_Empty
+  = name:HTML_Attribute_Name
+  { return [ name, true ] }
+
+HTML_Attribute_Unquoted
+  = name:HTML_Attribute_Name _* "=" _* value:$([a-zA-Z0-9]+)
+  { return [ name, value ] }
+
+HTML_Attribute_Quoted
+  = name:HTML_Attribute_Name _* "=" _* '"' value:$((!'"' .)*) '"'
+  { return [ name, value ] }
+  / name:HTML_Attribute_Name _* "=" _* "'" value:$((!"'" .)*) "'"
+  { return [ name, value ] }
+
+HTML_Attribute_Name
+  = $([a-zA-Z0-9:.]+)
 
 WP_Block_Type_Char
  = ASCII_AlphaNumeric
@@ -67,9 +83,6 @@ ASCII_AlphaNumeric
   / ASCII_Digit
   / Special_Chars
 
-WP_Block_Attribute_Value_Char
-  = [^ \t\r\n]
-
 ASCII_Letter
   = [a-zA-Z]
 

diff --git a/blocks/api/test/parser.js b/blocks/api/test/parser.js
@@ -11,7 +11,8 @@ import {
 	getBlockAttributes,
 	parseBlockAttributes,
 	createBlockWithFallback,
-	default as parse,
+	parseWithGrammar,
+	parseWithTinyMCE
 } from '../parser';
 import {
 	registerBlock,
@@ -139,106 +140,115 @@ describe( 'block parser', () => {
 	} );
 
 	describe( 'parse()', () => {
-		it( 'should parse the post content, including block attributes', () => {
-			registerBlock( 'core/test-block', {
-				// Currently this is the only way to test block content parsing?
-				attributes: function( rawContent ) {
-					return {
-						content: rawContent,
-					};
-				}
-			} );
-
-			const parsed = parse(
-				'<!-- wp:core/test-block smoked:yes -->' +
-				'Brisket' +
-				'<!-- /wp:core/test-block -->'
-			);
-
-			expect( parsed ).to.have.lengthOf( 1 );
-			expect( parsed[ 0 ].blockType ).to.equal( 'core/test-block' );
-			expect( parsed[ 0 ].attributes ).to.eql( {
-				content: 'Brisket',
-				smoked: 'yes',
-			} );
-			expect( parsed[ 0 ].uid ).to.be.a( 'string' );
-		} );
-
-		it( 'should parse the post content, ignoring unknown blocks', () => {
-			registerBlock( 'core/test-block', {
-				attributes: function( rawContent ) {
-					return {
-						content: rawContent + ' & Chicken'
-					};
-				}
-			} );
-
-			const parsed = parse(
-				'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
-				'<p>Broccoli</p>' +
-				'<!-- wp:core/unknown-block -->Ribs<!-- /wp:core/unknown-block -->'
-			);
-
-			expect( parsed ).to.have.lengthOf( 1 );
-			expect( parsed[ 0 ].blockType ).to.equal( 'core/test-block' );
-			expect( parsed[ 0 ].attributes ).to.eql( {
-				content: 'Ribs & Chicken'
-			} );
-			expect( parsed[ 0 ].uid ).to.be.a( 'string' );
-		} );
-
-		it( 'should parse the post content, using unknown block handler', () => {
-			registerBlock( 'core/test-block', {} );
-			registerBlock( 'core/unknown-block', {} );
-
-			setUnknownTypeHandler( 'core/unknown-block' );
-
-			const parsed = parse(
-				'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
-				'<p>Broccoli</p>' +
-				'<!-- wp:core/unknown-block -->Ribs<!-- /wp:core/unknown-block -->'
-			);
-
-			expect( parsed ).to.have.lengthOf( 3 );
-			expect( parsed.map( ( { blockType } ) => blockType ) ).to.eql( [
-				'core/test-block',
-				'core/unknown-block',
-				'core/unknown-block',
-			] );
-		} );
-
-		it( 'should parse the post content, including raw HTML at each end', () => {
-			registerBlock( 'core/test-block', {} );
-			registerBlock( 'core/unknown-block', {
-				// Currently this is the only way to test block content parsing?
-				attributes: function( rawContent ) {
-					return {
-						content: rawContent,
-					};
-				}
+		const parsers = { parseWithTinyMCE, parseWithGrammar };
+		Object.keys( parsers ).forEach( ( parser ) => {
+			const parse = parsers[ parser ];
+			describe( parser, () => {
+				it( 'should parse the post content, including block attributes', () => {
+					registerBlock( 'core/test-block', {
+						// Currently this is the only way to test block content parsing?
+						attributes: function( rawContent ) {
+							return {
+								content: rawContent,
+							};
+						}
+					} );
+
+					const parsed = parse(
+						'<!-- wp:core/test-block smoked="yes" url="http://google.com" chicken="ribs & \'wings\'" checked -->' +
+						'Brisket' +
+						'<!-- /wp:core/test-block -->'
+					);
+
+					expect( parsed ).to.have.lengthOf( 1 );
+					expect( parsed[ 0 ].blockType ).to.equal( 'core/test-block' );
+					expect( parsed[ 0 ].attributes ).to.eql( {
+						content: 'Brisket',
+						smoked: 'yes',
+						url: 'http://google.com',
+						chicken: 'ribs & \'wings\'',
+						checked: true
+					} );
+					expect( parsed[ 0 ].uid ).to.be.a( 'string' );
+				} );
+
+				it( 'should parse the post content, ignoring unknown blocks', () => {
+					registerBlock( 'core/test-block', {
+						attributes: function( rawContent ) {
+							return {
+								content: rawContent + ' & Chicken'
+							};
+						}
+					} );
+
+					const parsed = parse(
+						'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
+						'<p>Broccoli</p>' +
+						'<!-- wp:core/unknown-block -->Ribs<!-- /wp:core/unknown-block -->'
+					);
+
+					expect( parsed ).to.have.lengthOf( 1 );
+					expect( parsed[ 0 ].blockType ).to.equal( 'core/test-block' );
+					expect( parsed[ 0 ].attributes ).to.eql( {
+						content: 'Ribs & Chicken'
+					} );
+					expect( parsed[ 0 ].uid ).to.be.a( 'string' );
+				} );
+
+				it( 'should parse the post content, using unknown block handler', () => {
+					registerBlock( 'core/test-block', {} );
+					registerBlock( 'core/unknown-block', {} );
+
+					setUnknownTypeHandler( 'core/unknown-block' );
+
+					const parsed = parse(
+						'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
+						'<p>Broccoli</p>' +
+						'<!-- wp:core/unknown-block -->Ribs<!-- /wp:core/unknown-block -->'
+					);
+
+					expect( parsed ).to.have.lengthOf( 3 );
+					expect( parsed.map( ( { blockType } ) => blockType ) ).to.eql( [
+						'core/test-block',
+						'core/unknown-block',
+						'core/unknown-block',
+					] );
+				} );
+
+				it( 'should parse the post content, including raw HTML at each end', () => {
+					registerBlock( 'core/test-block', {} );
+					registerBlock( 'core/unknown-block', {
+						// Currently this is the only way to test block content parsing?
+						attributes: function( rawContent ) {
+							return {
+								content: rawContent,
+							};
+						}
+					} );
+
+					setUnknownTypeHandler( 'core/unknown-block' );
+
+					const parsed = parse(
+						'<p>Cauliflower</p>' +
+						'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
+						'<p>Broccoli</p>' +
+						'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
+						'<p>Romanesco</p>'
+					);
+
+					expect( parsed ).to.have.lengthOf( 5 );
+					expect( parsed.map( ( { blockType } ) => blockType ) ).to.eql( [
+						'core/unknown-block',
+						'core/test-block',
+						'core/unknown-block',
+						'core/test-block',
+						'core/unknown-block',
+					] );
+					expect( parsed[ 0 ].attributes.content ).to.eql( '<p>Cauliflower</p>' );
+					expect( parsed[ 2 ].attributes.content ).to.eql( '<p>Broccoli</p>' );
+					expect( parsed[ 4 ].attributes.content ).to.eql( '<p>Romanesco</p>' );
+				} );
 			} );
-
-			setUnknownTypeHandler( 'core/unknown-block' );
-
-			const parsed = parse(
-				'<p>Cauliflower</p>' +
-				'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
-				'<p>Broccoli</p>' +
-				'<!-- wp:core/test-block -->Ribs<!-- /wp:core/test-block -->' +
-				'<p>Romanesco</p>'
-			);
-
-			expect( parsed ).to.have.lengthOf( 5 );
-			expect( parsed.map( ( { blockType } ) => blockType ) ).to.eql( [
-				'core/unknown-block',
-				'core/test-block',
-				'core/unknown-block',
-				'core/test-block',
-				'core/unknown-block',
-			] );
-			expect( parsed[ 0 ].attributes.content ).to.eql( '<p>Cauliflower</p>' );
-			expect( parsed[ 2 ].attributes.content ).to.eql( '<p>Broccoli</p>' );
-			expect( parsed[ 4 ].attributes.content ).to.eql( '<p>Romanesco</p>' );
 		} );
 	} );
 } );
diff --git a/post-content.js b/post-content.js
@@ -45,7 +45,7 @@ window._wpGutenbergPost = {
 			'<ul><li>Ship early</li><li>Ship often</li><li>Listen to feedback from real people</li><li>Anticipate market direction</li></ul>',
 			'<!-- /wp:core/list -->',
 
-			'<!-- wp:core/embed url:https://www.youtube.com/watch?v=Nl6U7UotA-M -->',
+			'<!-- wp:core/embed url="https://www.youtube.com/watch?v=Nl6U7UotA-M" -->',
 			'<iframe width="560" height="315" src="//www.youtube.com/embed/Nl6U7UotA-M" frameborder="0" allowfullscreen></iframe>',
 			'<!-- /wp:core/embed -->',
 		].join( '' ),