Amphtml: Deprecate amp-script[development] in favor of [data-ampdevmode]
We currently have an attribute called data-amp-dev-mode which helps you suppress validation errors on an AMP page and marks the page as invalid.
amp-script has it's own development mode.
These should be the same ideally.
@Gregable @choumx @westonruter
nainar
All 16 comments
It's actually data-ampdevmode. It was proposed in https://github.com/ampproject/amphtml/issues/20974 and landed in https://github.com/ampproject/amphtml/pull/23802/commits/d48d6d2de4e1ea2f183a9735d65120a31b5e433a and https://github.com/ampproject/amphtml/pull/24000/commits/13bb6bcbc0732dd06e20c53a0d8ffadc3e5e795a.
westonruter
on 9 Oct 2019
To subsume amp-script[development] (#24497) we'd need data-ampdevmode to be invalid on any tag, not just the <html> tag. Looks like this is currently special-cased in the validator.
choumx
on 9 Oct 2019
When data-ampdevmode is absent on the root HTML element, then it has no effect on the page. In order for it to have an effect, it must be on _both_ the html element _and_ the element that you want to ignore validation errors for. So this would mean amp-script[data-ampdevmode] should also anly have an effect if the document also has html[data-ampdevmode]. This would ensure the validator marks the page as invalid due to being in dev mode.
westonruter
on 9 Oct 2019
@westonruter IOW, when data-ampdevmode is present on amp-script then amp-script should also treat it as equivalent to development?
Should the runtime preserve the idea of data-ampdevmode to all components? Then it wouldn't be necessary for additional attributes on each component?
honeybadgerdontcare
on 9 Oct 2019
it must be on both the html element and the element that you want to ignore validation errors for.
Yea, I was hoping to avoid that additional step. Might still be worth removing the inconsistency.
choumx
on 9 Oct 2019
IOW, when
data-ampdevmodeis present onamp-scriptthenamp-scriptshould also treat it as equivalent todevelopment?
Yes, if data-ampdevmode is on the amp-script _and_ if data-ampdevmode is on the html document element as well.
westonruter
on 10 Oct 2019
From reading this thread, I'm not sure I understand what the proposed change is. Can @nainar or @westonruter clarify?
Gregable
on 16 Oct 2019
Instead of dev mode being indicated by adding a development attribute to the amp-script element, indicate dev mode by adding a data-amp-dev-mode to amp-script _and_ the root html element.
westonruter
on 16 Oct 2019
- [ ] Deprecate
developmentattribute onamp-scriptin validator-amp-script.protoascii. - [ ] New way to enable "dev mode" on
amp-scriptelements:data-ampdevmodeattribute is present on both theamp-scriptelement and the document element (html tag). - [ ] Update documentation.
/to @samouri
choumx
on 29 Oct 2019
Why do we only want to indicate dev mode if data-ampdevmode is also on the root html element? Wouldn't it be more intuitive to either:
- Have
data-ampdevmodeon the main html element removes size limitations for allamp-scripttags. data-ampdevmodeon _just_ theamp-scriptelement counts as a validation error and will remove the size limitation for just that script (without also needing to be on the root html element).
I'm in favor of the former, since it reduces the amount of work for website publishers.
samouri
on 6 Nov 2019
2.
data-ampdevmodeon _just_ theamp-scriptelement counts as a validation error and will remove the size limitation for just that script (without also needing to be on the root html element).
This won't cause a validation error. When there is no data-ampdevmode on the root element, it is just considered like a regular data- attribute:
- Have
data-ampdevmodeon the main html element removes size limitations for allamp-scripttags.
I think it makes sense to have the requirement that data-ampdevmode be on _both_ the root and on each amp-script you want to exempt. Having the attribute on the root means that the page being served has to overall opt-in to dev mode and that someone couldn't (perhaps maliciously) figure out a way to inject a <amp-script data-ampdevmode> into the body. It's similar to how the amp-script-src meta tag is required to be in the head.
westonruter
on 7 Nov 2019
This won't cause a validation error. When there is no data-ampdevmode on the root element, it is just considered like a regular data- attribute:
We can make it a validation error for <amp-script>.
jridgewell
on 7 Nov 2019
I think it makes sense to have the requirement that data-ampdevmode be on both the root and on each amp-script you want to exempt
Hmm. My preferred option (1) was to actually require the attribute on _only_ the root element (thus enabling dev mode on all amp-script tags). What is the benefit of also requiring it on the individual amp-script?
samouri
on 7 Nov 2019
I suppose consistency with how it is used with the validator? If adding the attribute on one element exempts validation errors from being raised on it, then it would make sense that adding the attribute to data-ampdevmode would likewise exempt it from normal validation constraints.
westonruter
on 8 Nov 2019
The way it works in the validator is that you add data-ampdevmode to the root <html> tag first. Then, any other tag that includes data-ampdevmode will have any errors suppressed on it or it's descendants.
If you want to do:
<html amp data-ampdevmode>
<head>...</head>
<body data-ampdevmode>
... Anything goes in here ...
</body>
</html>
Then that would be consistent with the validator mode. The idea of requiring it twice is that you can decide which parts of the document you want errors for and which parts you don't. You can add it to the body and/or head (*) if you want to just disable all errors.
(*) This doesn't get rid of the mandatory document-level requirement errors, just the tag-level errors. Missing boilerplate will still be reported for example.
Gregable
on 8 Nov 2019
Thanks @Gregable for the full explanation. I'm happy with that behavior.
I'll make amp-script behave the same way in https://github.com/ampproject/amphtml/pull/25454.
samouri
on 8 Nov 2019
Related issues
devanes
路
3Comments
cvializ
路
3Comments
gmajoulet
路
3Comments
radiovisual
路
3Comments
sryze
路
3Comments
Most helpful comment
The way it works in the validator is that you add
data-ampdevmodeto the root<html>tag first. Then, any other tag that includesdata-ampdevmodewill have any errors suppressed on it or it's descendants.If you want to do:
Then that would be consistent with the validator mode. The idea of requiring it twice is that you can decide which parts of the document you want errors for and which parts you don't. You can add it to the body and/or head (*) if you want to just disable all errors.
(*) This doesn't get rid of the mandatory document-level requirement errors, just the tag-level errors. Missing boilerplate will still be reported for example.