Skip to content

amp-action: Support whitelist lookup in AmpDocShadow#26684

Merged
mdmower merged 2 commits intoampproject:masterfrom
mdmower:pr-action
Feb 22, 2020
Merged

amp-action: Support whitelist lookup in AmpDocShadow#26684
mdmower merged 2 commits intoampproject:masterfrom
mdmower:pr-action

Conversation

@mdmower
Copy link
Copy Markdown
Contributor

@mdmower mdmower commented Feb 9, 2020

New getMetaByName method supports retrieving meta content values in
shadow docs as well.

New getMetaByName method supports retrieving meta content values in
shadow docs as well.
@amp-owners-bot amp-owners-bot bot requested a review from samouri February 9, 2020 02:12
@mdmower mdmower changed the title amp-action: Support whitelist lookup in ShadowRoot amp-action: Support whitelist lookup in AmpDocShadow Feb 9, 2020
Copy link
Copy Markdown
Member

@samouri samouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@mdmower
Copy link
Copy Markdown
Contributor Author

mdmower commented Feb 14, 2020

@samouri Following up on discussion in #26729, do you think I should go ahead and drop this conditional check (revising any unit tests as necessary)?

if (!root.head && !(root instanceof ShadowRoot))

@samouri
Copy link
Copy Markdown
Member

samouri commented Feb 14, 2020

And assume that the head will always be there? Makes sense to me

It is safe to assume that either document.head is available if
AmpDocSingle/AmpDocFie, or that AmpDoc.meta_ is non-null if
AmpDocShadow.
const root = this.ampdoc.getRootNode();
if (!root.head && !(root instanceof ShadowRoot)) {
return null;
}
Copy link
Copy Markdown
Member

@samouri samouri Feb 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome that we no longer need to ship this code ⭐️

@mdmower mdmower merged commit 4c71f6c into ampproject:master Feb 22, 2020
robinvanopstal added a commit to jungvonmatt/amphtml that referenced this pull request Feb 24, 2020
* master: (41 commits)
  custom-element: Minor test improvements (ampproject#26923)
  amp-pixel: Minor test improvements (ampproject#26918)
  viewer: Minor test improvements (ampproject#26906)
  dom: Minor test improvements (ampproject#26913)
  amp-action: Support whitelist lookup in AmpDocShadow (ampproject#26684)
  ✨ Update amp-access-scroll (ampproject#26810)
  🚀 Remove doc css and base css from ESM build (ampproject#26889)
  📖 [amp-story-player] Initial docs (ampproject#26606)
  Amp consent restrict fullscreen prod flag (ampproject#26909)
  📖 Clarify SXG duration minimum (ampproject#26890)
  Improve test vendor requests macros (ampproject#26828)
  🚀 Move scroll left and top macros out of url-replacement-impl (ampproject#25594)
  Update consent string maximum size to 200 bytes (ampproject#26741)
  ✨[amp-story-player] Adds tap-to-next/previous story (ampproject#26865)
  update owners file with correct syntax (ampproject#26899)
  amp-sticky-ad: Fix unit test (ampproject#26855)
  Add performance metrics to README (ampproject#26891)
  🐛 Bug fix: check links test (ampproject#26739)
  ✨Idealmedia uniq ad (ampproject#25838)
  📦 Update dependency jsdom to v16.2.0 (ampproject#26591)
  ...
@mdmower mdmower deleted the pr-action branch March 16, 2020 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants