-
Notifications
You must be signed in to change notification settings - Fork 383
Description
Bug Description
Split out from #4190.
In #2500 the admin bar was suppressed from being shown when doing validation requests. Then in #3187 this was reverted because the addition of dev mode meant that the presence of the admin bar would no longer make pages invalid. Nevertheless, this had an unexpected consequence. When the <html> element has invalid attributes, these are never detected during validation because a validation request is made as an authenticated user, and thus the admin bar is present, and the admin bar entails dev mode being enabled. Since the data-ampdevmode attribute is on the html element, any invalid attributes on the element are ignored for validation purposes. This is not expected.
Steps to reproduce
Activate Twenty Twenty and add this plugin code:
add_filter(
'language_attributes',
function ( $attr ) {
if ( ! is_admin() ) {
$attr .= ' amp-version="1902151859190" onclick="alert(\'hello\')" ';
}
return $attr;
}
);This adds two invalid AMP attributes, and when I look at an AMP page while logged-in, I see them:
<html class="no-js" lang="en-US" amp-version="1902151859190" onclick="alert('hello')" amp="" data-ampdevmode="">Nevertheless, the admin bar shows ✅ for validity.
However, if I access the same AMP page while being logged-out, I see:
<html class="no-js" lang="en-US" amp="">Similarly, if I go to validate the URL I see no validation errors:
But the validation errors are there. It's just they are not being reported.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation brief
- We should capture whether the
htmlelement originally haddata-ampdevmodeand if so, ignore validation errors. Otherwise if it was added by the plugin to enable dev mode, then go ahead and report validation errors on the element even though it has the attribute. This would be a special case unique for thehtmlelement.
