amphtml icon indicating copy to clipboard operation
amphtml copied to clipboard

Validator no longer detects missing body/head tags

Open samhutchins opened this issue 2 years ago • 2 comments

Description

I have a snapshot of validator.js from 2021/09/14 which correctly identifies a missing head tag, but the current validator_wasm.js version doesn't report the missing head tag, only the missing children of the head tag

I have also noticed a difference in the way a missing body tag is reported. In that snapshot it reported it as a MANDATORY_TAG_MISSING error, but validator_wasm.js reports it as DISALLOWED_MANUFACTURED_BODY

Are either of these changes intentional?

Reproduction Steps

Snapshot of validator.js: validator.js.zip Snapshot of validator_wasm.js: validator_wasm.js.zip

Missing head: index.html.zip Missing body: index.html.zip

Missing head: Run amp.validator.validateString on the provided test page Expected:

...
            {
                "severity": "ERROR",
                "code": "MANDATORY_TAG_MISSING",
                "line": 7,
                "col": 6,
                "specUrl": "https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml/#required-markup",
                "params": [
                    "head"
                ],
                "category": null,
                "dataAmpReportTestValue": null
            },
...

Actual: No missing head is reported

Missing body: Run amp.validator.validateString on the provided test page expected:

...
            {
                "severity": "ERROR",
                "code": "MANDATORY_TAG_MISSING",
                "line": 11,
                "col": 6,
                "specUrl": "https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml/#required-markup",
                "params": [
                    "body"
                ],
                "category": null,
                "dataAmpReportTestValue": null
            }
...

Actual:

...
            {
                "severity": "ERROR",
                "code": "DISALLOWED_MANUFACTURED_BODY",
                "line": 10,
                "col": 11,
                "paramsList": [],
                "params": [],
                "_PB_BASE64": "MAEIQBAKGAs="
            }
...

Relevant Logs

No response

Browser(s) Affected

No response

OS(s) Affected

No response

Device(s) Affected

No response

AMP Version Affected

No response

samhutchins avatar Jul 18 '22 12:07 samhutchins

I don't know whether these are particularly problematic -- @amaltas or @MichaelRybak do you have thoughts?

newmuis avatar Aug 10 '22 15:08 newmuis

I've confirmed validator currently allows missing

tag, as well as just a missing closing tag. Note: It also allows other missing closing tags, e.g. missing

or .

@amaltas @gmajoulet @alanorozco folks, what's the impact of this bug?

MichaelRybak avatar Aug 11 '22 14:08 MichaelRybak