🐛 Enforce amp-ad type=custom has an HTTPS data-url#23386
🐛 Enforce amp-ad type=custom has an HTTPS data-url#23386jridgewell merged 4 commits intoampproject:masterfrom
Conversation
| # extensions/amp-auto-ads/*/placement.js | ||
| disallowed_ancestor: "AMP-APP-BANNER" | ||
| attrs: { | ||
| name: "data-uri" |
There was a problem hiding this comment.
cc @lannka Do you expect this to break any current usage of custom ad? I think it's fine since we document that data-url is a mandatory attribute.
There was a problem hiding this comment.
There are two new requirements if type=custom
data-urlis presentdata-urlvalue ishttps
Are you confident that both of those are sufficiently followed in the wild.
There was a problem hiding this comment.
No, but the docs always said it had to be an HTTPS url.
| ); | ||
|
|
||
| const urlService = Services.urlForDoc(this.element); | ||
| userAssert( |
There was a problem hiding this comment.
QQ: Is the check necessary if we already have
value_url: {
protocol: "https"
}
in validator check
There was a problem hiding this comment.
User asserts are for local development, too.
| attrs: { | ||
| name: "type" | ||
| mandatory: true | ||
| blacklisted_value_regex: "^(" |
There was a problem hiding this comment.
Remove this. See comment below.
| attrs: { | ||
| name: "type" | ||
| mandatory: true | ||
| value: "custom" |
There was a problem hiding this comment.
add:
dispatch_key: NAME_VALUE_DISPATCH
The dispatch_key will cause any AMP-AD tags with an attribute of type=custom to only be matched against this one tagspec.
| # extensions/amp-auto-ads/*/placement.js | ||
| disallowed_ancestor: "AMP-APP-BANNER" | ||
| attrs: { | ||
| name: "data-uri" |
There was a problem hiding this comment.
There are two new requirements if type=custom
data-urlis presentdata-urlvalue ishttps
Are you confident that both of those are sufficiently followed in the wild.
* Enforce that <amp-ad type=custom> has an HTTPS data-uri * Validator: Ensure amp-ad type=custom has a HTTPS data-url * validator proto fixes * Fix validator tests
* cl/258377914 Revision bump for #23122 * cl/258634966 Revision bump for #23349 * cl/258870451 Fix incorrect allowance of `<form method="get">` with relative URLs * cl/259565186 Revision bump for #23386 * cl/259587993 Add a descriptive comment to amp-carousel rules. * cl/259661215 Fix #23012 by removing the dispatch key on amp-carousel type=carousel. * cl/259662509 Revision bump for #23148 * cl/259988931 Revision bump for #23482
Fixes #21314