ampdoc: Add get/setMetaByName methods #26609
Conversation
- Add getMetaByName to AmpDoc with overrides implemented in AmpDocSingle and AmpDocShadow to get the content value of a meta[name] tag. - Add setMetaByName ot AmpDoc with override implemented in AmpDocShadow to store meta[name] tag content values.
|
/cc @choumx Is this close to what you had in mind? |
src/service/ampdoc-impl.js
Outdated
| const mName = el.getAttribute('name'); | ||
| let sanitizedName; | ||
| try { | ||
| sanitizedName = mName.replace(/[^\x00-\x7F]/gu, '_').toLowerCase(); |
There was a problem hiding this comment.
This is overkill. If a publisher puts Unicode characters in the value of name, then unexpected behavior is reasonable (like __ instead of _). Can just do sanitizedName = mName.replace(/[^\x00-\x7F]/g, '_').toLowerCase(); in all browsers.
There was a problem hiding this comment.
Or maybe any sanitization at all is overkill. Thoughts?
There was a problem hiding this comment.
Woo I appreciate the thoroughness!
Yea I think it's a bit overkill since this isn't a public API and there's a short list of valid meta[name] values (all of which are just ASCII). Probably better to save the bytes and CPU for users e.g. just use meta[name="..."] query selector.
src/service/ampdoc-impl.js
Outdated
| // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/unicode | ||
| let sanitizedName; | ||
| try { | ||
| sanitizedName = name.replace(/[^\x00-\x7F]/gu, '_').toLowerCase(); |
src/service/ampdoc-impl.js
Outdated
| const mName = el.getAttribute('name'); | ||
| let sanitizedName; | ||
| try { | ||
| sanitizedName = mName.replace(/[^\x00-\x7F]/gu, '_').toLowerCase(); |
There was a problem hiding this comment.
Woo I appreciate the thoroughness!
Yea I think it's a bit overkill since this isn't a public API and there's a short list of valid meta[name] values (all of which are just ASCII). Probably better to save the bytes and CPU for users e.g. just use meta[name="..."] query selector.
src/service/ampdoc-impl.js
Outdated
| * @return {?string} | ||
| */ | ||
| getMetaByName(unusedName) { | ||
| return /** @type {?} */ (devAssert(null, 'not implemented')); |
There was a problem hiding this comment.
Should querying the headNode() be the default implementation and overriden only by AmpDocShadow?
src/service/ampdoc-impl.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Stores the value of an ampdoc's meta tag content for a given name. |
There was a problem hiding this comment.
Let's add some language that dissuades users from calling this. It should only be used during setup of the AmpDoc.
- Simplify meta name/content lookups - Add test for non-shadow getMetaByName
|
Here is a simple |
|
@mdmower Looks like we're good, just needed to push a blank commit |
Thanks for your help @micajuine-ho ! |
|
@ampproject/wg-runtime or @ampproject/wg-performance could you please review this PR and the bundle size increase? For context, see #26535 (comment) . Thanks! |
getMetaByNametoAmpDocto get thecontentvalue from a<meta name=... content=...>element. Override inAmpShadowDocto perform the lookup in a name/content pair dictionary.setMetaByNametoAmpDoc(only implemented inAmpDocShadow) to store meta name/content values in a variable. This enables retention of information stored in<meta name=... content=...>tags since it would be invalid HTML to copy them into theShadowRootwhere there is no<head>under which they could reside.