Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌸 Cherry-pick request for #25332 into #25222 (Approved) #25338

Closed
jridgewell opened this issue Oct 30, 2019 · 2 comments
Closed

🌸 Cherry-pick request for #25332 into #25222 (Approved) #25338

jridgewell opened this issue Oct 30, 2019 · 2 comments
Labels

Comments

@jridgewell
Copy link
Contributor

jridgewell commented Oct 30, 2019

Cherry-pick request

Issue PR Production? RC? Release issue
#25332 #25337 Yes No #25222
#25332 #25337 No Yes #25319

Why does this issue meet the cherry-pick criteria?

Custom Elements inside Shadow Roots are not upgraded.

Mini-postmortem

Summary

Custom Elements inside a Shadow Root's tree (not in the document's light DOM) failed to upgrade in a very specific series:

  1. Any custom is registered (eg, <amp-img>).
  2. A shadow root is created
  3. An unregistered custom element is appended to the shadow root (eg, <amp-state>)
  4. That new custom element is registered
Users affected Impact
PWA pages using amp-shadow Previously unregistered elements failed to upgrade

Root Causes

  1. When creating a new shadow root, the Custom Element polyfill adds the shadow root to a list of tracked "root" trees
  2. When the first custom element (any custom element) is registered, we cleared the list of root trees
  3. When registering any more custom elements, we used a querySelectorAll on the document to search for any already-existing elements to upgrade

Because querySelectorAll does not return results inside a shadow tree, we failed to find already-existing CEs inside them.

Action Items

Action Item Type Owner PR #
A test suite for custom elements Prevent @jridgewell #25370

Lessons Learned

Things that went well

  • The issue was reported within 1 day of release

Things that went wrong

  • Releasing the patched version took an additional day due to infrastructure issues

/cc @ampproject/wg-approvers @ampproject/cherry-pick-approvers

@jridgewell jridgewell added the Type: Release Used to track AMP releases from canary to production label Oct 30, 2019
@cramforce
Copy link
Member

Approved

@jridgewell jridgewell changed the title 🌸 Cherry-pick request for #25332 into #25222 🌸 Cherry-pick request for #25332 into #25222 (Approved) Oct 30, 2019
@jridgewell
Copy link
Contributor Author

Postmortem posted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants