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

Migrate deprecated web-view method #11798

Merged
merged 1 commit into from Feb 1, 2018

Conversation

Projects
None yet
4 participants
@codebytere
Copy link
Member

codebytere commented Feb 1, 2018

Closes #11788.

Migrates call to createShadowRoot() in web-view.js to attachShadow(shadowRootInit) per Mozilla Docs

@codebytere codebytere requested a review from electron/reviewers as a code owner Feb 1, 2018

@zeke
Copy link
Member

zeke left a comment

Any idea how to write a test for this?

I'm not familiar with this code, but I think it might be safer to first check for the attachShadow method on the node, falling back to createShadowRoot in its absence.

@codebytere

This comment has been minimized.

Copy link
Member Author

codebytere commented Feb 1, 2018

@zeke from a compat standpoint attachShadow is available on more browsers than createShadowRoot so i'm not sure there'd be an instance where it would need to fall back; i have no idea how to test specifically, but it's integral to the event handlers for web-view so if it wasn't working all those tests would fail i'm 99% sure

@ckerr

ckerr approved these changes Feb 1, 2018

Copy link
Member

ckerr left a comment

👍

@ckerr ckerr merged commit b32a7d4 into master Feb 1, 2018

9 checks passed

ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64-test Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@ckerr ckerr deleted the shadowroot-deprecate-fix branch Feb 1, 2018

@SMotaal

This comment has been minimized.

Copy link

SMotaal commented Mar 7, 2018

This change means that it will only be possible to create v1 shadow roots, even though v0 is still supported by Chrome.

As far as my tests show, the only way to extend builtin elements in Chrome (no one else allows it) is through Custom Elements v0 (ie. document.registerElement). Unless a v1 Shadow will function normally on v0 Elements, then I wonder if this PR will break the ability to extend builtin elements in the near future (with Web Component specs in upstream limbo).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment