From 8513f8d3c6ae73075fcb5c7e909e4fb19bbb16d7 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 14:53:11 +0530 Subject: [PATCH 1/4] Make validation case insensitive in all scenarios by converting everything to lowercase --- src/Pass/AmpImgFixPass.php | 2 +- src/Pass/PreliminaryPass.php | 3 ++- src/Pass/StandardFixPass.php | 2 +- src/Pass/StandardFixPassTwo.php | 2 +- src/Validate/Context.php | 7 ++++--- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Pass/AmpImgFixPass.php b/src/Pass/AmpImgFixPass.php index 04c48def..ec9d6b55 100644 --- a/src/Pass/AmpImgFixPass.php +++ b/src/Pass/AmpImgFixPass.php @@ -50,7 +50,7 @@ function pass() if (in_array($error->code, [ValidationErrorCode::INCONSISTENT_UNITS_FOR_WIDTH_AND_HEIGHT, ValidationErrorCode::MANDATORY_ATTR_MISSING, ValidationErrorCode::INVALID_ATTR_VALUE, ValidationErrorCode::IMPLIED_LAYOUT_INVALID, ValidationErrorCode::SPECIFIED_LAYOUT_INVALID]) && !$error->resolved && !empty($error->dom_tag) && - $error->dom_tag->tagName == 'amp-img' + strtolower($error->dom_tag->tagName) == 'amp-img' ) { $amp_img_el = new DOMQuery($error->dom_tag); diff --git a/src/Pass/PreliminaryPass.php b/src/Pass/PreliminaryPass.php index 851ecff4..214095a9 100644 --- a/src/Pass/PreliminaryPass.php +++ b/src/Pass/PreliminaryPass.php @@ -49,7 +49,8 @@ public function pass() $dom_el = $el->get(0); $context_string = $this->getContextString($dom_el); $lineno = $this->getLineNo($dom_el); - $message = "$dom_el->tagName tag matches CSS selector '$remove_this'"; + $tagname = mb_strtolower($dom_el->tagName, 'UTF-8'); + $message = "$tagname tag matches CSS selector '$remove_this'"; $this->addActionTaken(new ActionTakenLine($message, ActionTakenType::BLACKLISTED_TAG_REMOVED, $lineno, $context_string)); $el->remove(); } diff --git a/src/Pass/StandardFixPass.php b/src/Pass/StandardFixPass.php index deb82139..c5771d8d 100644 --- a/src/Pass/StandardFixPass.php +++ b/src/Pass/StandardFixPass.php @@ -79,7 +79,7 @@ public function pass() continue; } - $tag_name = $error->dom_tag->tagName; + $tag_name = mb_strtolower($error->dom_tag->tagName, 'UTF-8'); // Property value pairs if (in_array($error->code, $this->remove_properties_for_codes) diff --git a/src/Pass/StandardFixPassTwo.php b/src/Pass/StandardFixPassTwo.php index a1ce2353..433e886b 100644 --- a/src/Pass/StandardFixPassTwo.php +++ b/src/Pass/StandardFixPassTwo.php @@ -78,7 +78,7 @@ public function pass() if ($test_validation_result->status !== ValidationResultStatus::PASS && in_array('head', $local_context->getAncestorTagNames()) ) { - if ($error->dom_tag->tagName !== 'style' || !$error->dom_tag->hasAttribute('amp-custom')) { + if (strtolower($error->dom_tag->tagName) !== 'style' || !$error->dom_tag->hasAttribute('amp-custom')) { $error->dom_tag->parentNode->removeChild($error->dom_tag); $error->addGroupActionTaken(new ActionTakenLine($error->dom_tag->nodeName, ActionTakenType::TAG_REMOVED_FROM_HEAD_AFTER_REVALIDATE_FAILED)); } diff --git a/src/Validate/Context.php b/src/Validate/Context.php index 2c7a505f..5fc66683 100644 --- a/src/Validate/Context.php +++ b/src/Validate/Context.php @@ -244,7 +244,7 @@ protected function setAncestorTagNames() $ancestor_tag_names = []; $tag = $this->dom_tag; while (($tag = $tag->parentNode) && !empty($tag->tagName)) { - $ancestor_tag_names[] = $tag->tagName; + $ancestor_tag_names[] = mb_strtolower($tag->tagName, 'UTF-8');; } $ancestor_tag_names[] = '!doctype'; $this->ancestor_tag_names = $ancestor_tag_names; @@ -277,7 +277,7 @@ protected function setParentTagName() if (empty($this->dom_tag->parentNode->tagName)) { $this->parent_tag_name = '!doctype'; } else { - $this->parent_tag_name = $this->dom_tag->parentNode->tagName; + $this->parent_tag_name = mb_strtolower($this->dom_tag->parentNode->tagName, 'UTF-8'); } } @@ -377,7 +377,8 @@ public function getContextString(\DOMElement $dom_el) /** @var string[] $attributes */ $attributes = $this->encounteredAttributes($dom_el); - $context_string = "<$dom_el->tagName"; + $tagname = mb_strtolower($dom_el->tagName, 'UTF-8'); + $context_string = "<$tagname"; foreach ($attributes as $attr_name => $attr_value) { // Skip embedded line numbers if ($attr_name == AMP::AMP_LINENUM_ATTRIBUTE) { From 165695c8bc3764ec8f6beb57b6ecbe6bfada018b Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 15:08:44 +0530 Subject: [PATCH 2/4] More changes related to making validator case insensitive in all scenarios (by using lowercase everywhere) --- src/Pass/MinimumValidFixPass.php | 3 ++- src/Pass/StandardFixPassTwo.php | 6 ++++-- src/Pass/StandardScanPass.php | 3 ++- src/Validate/Context.php | 4 +++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Pass/MinimumValidFixPass.php b/src/Pass/MinimumValidFixPass.php index 5fb3bee5..f845988b 100644 --- a/src/Pass/MinimumValidFixPass.php +++ b/src/Pass/MinimumValidFixPass.php @@ -107,7 +107,8 @@ public function pass() /** @var \DOMElement $tag */ foreach ($all_tags as $tag) { $local_context->attachDomTag($tag); - $this->parsed_rules->validateTag($local_context, $tag->nodeName, $this->encounteredAttributes($tag), $temp_validation_result); + $tagname = mb_strtolower($tag->tagName, 'UTF-8'); + $this->parsed_rules->validateTag($local_context, $tagname, $this->encounteredAttributes($tag), $temp_validation_result); $this->parsed_rules->validateTagOnExit($local_context, $temp_validation_result); $local_context->detachDomTag(); } diff --git a/src/Pass/StandardFixPassTwo.php b/src/Pass/StandardFixPassTwo.php index 433e886b..82fee3df 100644 --- a/src/Pass/StandardFixPassTwo.php +++ b/src/Pass/StandardFixPassTwo.php @@ -68,7 +68,8 @@ public function pass() $test_validation_result->status = ValidationResultStatus::UNKNOWN; $local_context->attachDomTag($error->dom_tag); - $this->parsed_rules->validateTag($local_context, $error->dom_tag->nodeName, $this->encounteredAttributes($error->dom_tag), $test_validation_result); + $tagname = mb_strtolower($error->dom_tag->tagName, 'UTF-8'); + $this->parsed_rules->validateTag($local_context, $tagname, $this->encounteredAttributes($error->dom_tag), $test_validation_result); $this->parsed_rules->validateTagOnExit($local_context, $test_validation_result); if ($test_validation_result->status == ValidationResultStatus::UNKNOWN) { @@ -80,7 +81,8 @@ public function pass() ) { if (strtolower($error->dom_tag->tagName) !== 'style' || !$error->dom_tag->hasAttribute('amp-custom')) { $error->dom_tag->parentNode->removeChild($error->dom_tag); - $error->addGroupActionTaken(new ActionTakenLine($error->dom_tag->nodeName, ActionTakenType::TAG_REMOVED_FROM_HEAD_AFTER_REVALIDATE_FAILED)); + $tagname = mb_strtolower($error->dom_tag->tagName, 'UTF-8'); + $error->addGroupActionTaken(new ActionTakenLine($tagname, ActionTakenType::TAG_REMOVED_FROM_HEAD_AFTER_REVALIDATE_FAILED)); } } diff --git a/src/Pass/StandardScanPass.php b/src/Pass/StandardScanPass.php index bc8ac48e..1ac96ad1 100644 --- a/src/Pass/StandardScanPass.php +++ b/src/Pass/StandardScanPass.php @@ -41,7 +41,8 @@ public function pass() foreach ($all_tags as $tag) { $count++; $this->context->attachDomTag($tag); - $this->parsed_rules->validateTag($this->context, $tag->nodeName, $this->encounteredAttributes($tag), $this->validation_result); + $tagname = mb_strtolower($tag->tagName, 'UTF-8'); + $this->parsed_rules->validateTag($this->context, $tagname, $this->encounteredAttributes($tag), $this->validation_result); $this->parsed_rules->validateTagOnExit($this->context, $this->validation_result); $this->context->detachDomTag(); } diff --git a/src/Validate/Context.php b/src/Validate/Context.php index 5fc66683..025e7f19 100644 --- a/src/Validate/Context.php +++ b/src/Validate/Context.php @@ -256,7 +256,9 @@ protected function setChildTagNames() /** @var \DOMNode $child_node */ foreach ($this->dom_tag->childNodes as $child_node) { if ($child_node->nodeType == XML_ELEMENT_NODE) { - $this->child_tag_names[] = $child_node->nodeName; + /** @var \DOMElement $child_node */ + $tagname = mb_strtolower($child_node->tagName, 'UTF-8'); + $this->child_tag_names[] = $tagname; } } } From 0a35986a38e93291371ab49aa5515ea618d5e00e Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 15:12:39 +0530 Subject: [PATCH 3/4] Update the svg.html test. This is now a test for case insenstivity also --- tests/README.md | 3 +-- tests/test-data/full-html/svg.html | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/README.md b/tests/README.md index 9511f95e..df618e11 100644 --- a/tests/README.md +++ b/tests/README.md @@ -24,8 +24,7 @@ Note: An explicit commit is noted as these files can change. There is a slight custom modification made by us to this file (see our repo's 36c67aace ) and that is to close the `` tags in the file that are not closed. This causes problems for PHP dom. * `test-data/full-html/several_errors.html` from https://github.com/ampproject/amphtml/blob/96335e0540532264b3b38d498070f17473692b21/validator/testdata/feature_tests/several_errors.html * `test-data/full-html/spec_example.html` https://github.com/ampproject/amphtml/blob/96335e0540532264b3b38d498070f17473692b21/validator/testdata/feature_tests/spec_example.html -* `test-data/full-html/svg.html` https://github.com/ampproject/amphtml/blob/96335e0540532264b3b38d498070f17473692b21/validator/testdata/feature_tests/svg.html - Small modification: css id `svg_3` was repeated and has been made unique by changing line 68 to `another_svg_circle`. +* `test-data/full-html/svg.html` https://github.com/ampproject/amphtml/raw/8ab5d550fae93b9a1bb8a06d9fb82ffc08569b44/validator/testdata/feature_tests/svg.html * `test-data/full-html/track_tag.html` https://github.com/ampproject/amphtml/blob/27ee29ffc3d809fcc8143044d22df9d176ad8169/validator/testdata/feature_tests/track_tag.html * `test-data/full-html/urls.html` from https://github.com/ampproject/amphtml/blob/eddc6fd2224559cb7ccc6a1e27484e52de3d9301/validator/testdata/feature_tests/urls.html * `test-data/full-html/validator-amp-accordion.html` https://github.com/ampproject/amphtml/blob/27ee29ffc3d809fcc8143044d22df9d176ad8169/extensions/amp-accordion/0.1/test/validator-amp-accordion.html diff --git a/tests/test-data/full-html/svg.html b/tests/test-data/full-html/svg.html index 14984597..8cb6373a 100644 --- a/tests/test-data/full-html/svg.html +++ b/tests/test-data/full-html/svg.html @@ -65,7 +65,22 @@

SVG

stuff in HTML5, we allow it just like browsers would do in practice. --> - + + + + + + + + + + + + + + + From 1d7b8ad6310f97e5cb72206d88e014ddc11a7636 Mon Sep 17 00:00:00 2001 From: Sidharth Kshatriya Date: Tue, 5 Jul 2016 15:33:29 +0530 Subject: [PATCH 4/4] regen svg.html test expected output --- tests/test-data/full-html/svg.html.out | 40 ++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/tests/test-data/full-html/svg.html.out b/tests/test-data/full-html/svg.html.out index 2b226d9d..4f3d5b6a 100644 --- a/tests/test-data/full-html/svg.html.out +++ b/tests/test-data/full-html/svg.html.out @@ -62,8 +62,23 @@ stuff in HTML5, we allow it just like browsers would do in practice. --> - + + + + + + + + + + + + + + + @@ -138,11 +153,26 @@ Line 64: output from graphics tools. While it's a bit weird to have this na Line 65: stuff in HTML5, we allow it just like browsers would do in practice. Line 66: --> Line 67: -Line 68: +Line 68: Line 69: -Line 70: -Line 71: -Line 72: +Line 70: +Line 71: +Line 73: +Line 74: +Line 75: +Line 76: +Line 77: +Line 78: +Line 79: +Line 80: +Line 81: +Line 82: +Line 83: +Line 84: +Line 85: +Line 86: +Line 87: