Skip to content

Commit

Permalink
Limit noscript placeholder replacements to the HEAD on old versions o…
Browse files Browse the repository at this point in the history
…f libxml
  • Loading branch information
westonruter committed Jun 26, 2018
1 parent b0a08e1 commit fb55f08
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
19 changes: 14 additions & 5 deletions includes/utils/class-amp-dom-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,22 @@ public static function get_dom( $document ) {
* When appearing in the head element, a noscript can cause the head to close prematurely
* and the noscript gets moved to the body and anything after it which was in the head.
* See <https://stackoverflow.com/questions/39013102/why-does-noscript-move-into-body-tag-instead-of-head-tag>.
* This is limited to only running in the head element because this is where the problem lies,
* and it is important for the AMP_Script_Sanitizer to be able to access the noscript elements
* in the body otherwise.
*/
$document = preg_replace_callback(
'#<noscript[^>]*>.*?</noscript>#si',
function( $matches ) {
$placeholder = sprintf( '<!--noscript:%s-->', (string) wp_rand() );
AMP_DOM_Utils::$noscript_placeholder_comments[ $placeholder ] = $matches[0];
return $placeholder;
'#^.+?(?=<body)#is',
function( $head_matches ) {
return preg_replace_callback(
'#<noscript[^>]*>.*?</noscript>#si',
function( $noscript_matches ) {
$placeholder = sprintf( '<!--noscript:%s-->', (string) wp_rand() );
AMP_DOM_Utils::$noscript_placeholder_comments[ $placeholder ] = $noscript_matches[0];
return $placeholder;
},
$head_matches[0]
);
},
$document
);
Expand Down
3 changes: 2 additions & 1 deletion tests/test-amp-script-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public function get_noscript_data() {
* @covers AMP_Script_Sanitizer::sanitize()
*/
public function test_noscript_promotion( $source, $expected = null ) {
$dom = AMP_DOM_Utils::get_dom_from_content( $source );
$dom = AMP_DOM_Utils::get_dom_from_content( $source );
$this->assertSame( 1, $dom->getElementsByTagName( 'noscript' )->length );
$sanitizer = new AMP_Script_Sanitizer( $dom );
$sanitizer->sanitize();
$whitelist_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom );
Expand Down

0 comments on commit fb55f08

Please sign in to comment.