Have GuestViewContainer use Shadow DOM v1 |
|||||||
Issue descriptionGuestViewContainer creates a shadow DOM in which the internal element is placed. We're currently calling |createShadowRoot| on the container element, which means we're using Shadow DOM v0. We should use |attachShadow| for Shadow DOM v1 instead.
,
Jan 24 2018
+kochi who's looking at deprecating Shadow DOM v0.
,
Jan 26 2018
,
Apr 19 2018
Is this still true?
,
Apr 19 2018
Re c#4: Yes. The relevant code is at https://cs.chromium.org/chromium/src/extensions/renderer/resources/guest_view/guest_view_container.js?rcl=8bc1641aca75f25f995b55910f2b4e94562cb24f&l=28 .
,
Apr 23 2018
Thanks, I haven't figured out the implication of using shadow dom in guest view container, so I should be missing something. Tentatively changing createShadowRoot to attachShadow to see what breaks at: https://chromium-review.googlesource.com/c/chromium/src/+/1023677
,
Apr 23 2018
Looks like the following tests failed. WebViewTests/WebViewAccessibilityTest.FocusAccessibility/1 WebViewTests/WebViewAccessibilityTest.FocusAccessibility/0 WebViewInteractiveTests/WebViewFocusInteractiveTest.Focus_AdvanceFocus/1 WebViewInteractiveTests/WebViewFocusInteractiveTest.FocusAndVisibility/0 WebViewInteractiveTests/WebViewFocusInteractiveTest.FocusAndVisibility/1
,
May 15 2018
It seems the guest view container looks like using Custom Elements V0 as well. https://cs.chromium.org/chromium/src/extensions/renderer/resources/guest_view/guest_view_container.js?l=209 It has to be migrated to Custom Elements v1 API as well. Does anyone in the webview team has any plan working on this?
,
May 15 2018
Re c#9: Once I finish with the Site Isolation input issues I'm currently working on, I can take a look at this. Are there any resources describing the differences in behaviour between the v0 and v1 versions of Shadow DOM and Custom Elements? From c#7, it looks like there is a difference in how focus is handled between Shadow DOM v0 and v1.
,
May 31 2018
Sorry not revisiting this for a while. https://hayato.io/2016/shadowdomv1/ would be a good resource to start with for migration from Shadow DOM v0 to v1. I'm not sure the real reason for the failures in #c7, maybe something was not rendered.
,
Jul 4
,
Jul 26
Adding to this, for eventually removing all Shadow DOM V0 and Custom Elements V0 infrastructure from Blink, <webview> needs to migrate from Custom Elements V0 to Custom Elements V1.
,
Jul 26
Filed issue 867831 for comment13.
,
Aug 10
So there appears to be a behaviour change between Shadow DOM v0 and v1 regarding tabindex. Currently the element to which we attach the shadow dom has tabindex=-1. The contents of the frame in the shadow dom are reachable with v0, but they're unreachable with v1. If I remove the tabindex, keyboard focus works properly with v1. Fortunately, it looks like the tabindex on the container is no longer necessary.
,
Aug 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4caaa081a11650f79f34786d5a8ff1c923ed626f commit 4caaa081a11650f79f34786d5a8ff1c923ed626f Author: Kevin McNee <mcnee@chromium.org> Date: Fri Aug 10 19:44:49 2018 Remove tabindex from the GuestViewContainer element. A tabindex of -1 on the GuestViewContainer element allows it to be focusable but not reachable via sequential keyboard navigation. There does not appear to be any further need for the GuestViewContainer element itself to be focusable given that we override the focus method. Previously, we had a focus event listener on the GuestViewContainer element in order to propagate the focus to the internal element which would explain why this was needed. See https://crrev.com/6476573b7d9099b04002736d90a078550c8651eb https://crrev.com/53ba951bb6ed4df0998240520fed9d00a7ffff67 Bug: 231664 , 803274 Change-Id: If6ffb6f7282db86a1a7ab9e856785e6f51d79a80 Reviewed-on: https://chromium-review.googlesource.com/1170988 Reviewed-by: Ehsan Karamad <ekaramad@chromium.org> Commit-Queue: Kevin McNee <mcnee@chromium.org> Cr-Commit-Position: refs/heads/master@{#582294} [modify] https://crrev.com/4caaa081a11650f79f34786d5a8ff1c923ed626f/extensions/renderer/resources/guest_view/guest_view_container.js
,
Aug 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e61fa07cd555ca35a730605c75725f6db5bb8698 commit e61fa07cd555ca35a730605c75725f6db5bb8698 Author: Kevin McNee <mcnee@chromium.org> Date: Mon Aug 13 17:06:10 2018 Migrate GuestViewContainer to Shadow DOM v1 Bug: 803274 Change-Id: If56ee6ad0220b429b4497ccecef90eb257265478 Reviewed-on: https://chromium-review.googlesource.com/1170989 Commit-Queue: Kevin McNee <mcnee@chromium.org> Reviewed-by: James MacLean <wjmaclean@chromium.org> Cr-Commit-Position: refs/heads/master@{#582621} [modify] https://crrev.com/e61fa07cd555ca35a730605c75725f6db5bb8698/extensions/renderer/resources/guest_view/guest_view_container.js
,
Aug 13
,
Aug 14
So now that we're using v1, does anyone have any thoughts about whether we should make the guest view container's shadow root closed? It seems like the only benefit would be indicating our intent that guest view elements' shadow roots should be treated like user agent shadow roots. As mentioned in the above migration doc, it doesn't offer any guarantees since Element.prototype.attachShadow could be clobbered by user scripts.
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/058930e68573f2dd66725d0145277af89f35a576 commit 058930e68573f2dd66725d0145277af89f35a576 Author: Kevin McNee <mcnee@chromium.org> Date: Thu Nov 29 16:59:07 2018 Close the GuestView shadow root Now that we're using Shadow DOM v1, we have the option to make the shadow DOM unreachable by outside script by specifying a closed encapsulation mode. Now GuestViews will behave more like other elements defined by the browser which internally use shadow DOM. This also prevents script from interfering with our internal elements and producing unexpected behaviour. Bug: 892886, 803274 Change-Id: I57b2513e3ff7290e286b3d8391e7d93bac2bc4e9 Reviewed-on: https://chromium-review.googlesource.com/c/1342768 Commit-Queue: Kevin McNee <mcnee@chromium.org> Reviewed-by: Ehsan Karamad <ekaramad@chromium.org> Cr-Commit-Position: refs/heads/master@{#612232} [modify] https://crrev.com/058930e68573f2dd66725d0145277af89f35a576/chrome/test/data/extensions/platform_apps/web_view/shim/main.js [modify] https://crrev.com/058930e68573f2dd66725d0145277af89f35a576/extensions/browser/guest_view/web_view/web_view_apitest.cc [modify] https://crrev.com/058930e68573f2dd66725d0145277af89f35a576/extensions/renderer/resources/guest_view/guest_view_container.js [modify] https://crrev.com/058930e68573f2dd66725d0145277af89f35a576/extensions/test/data/web_view/apitest/main.js |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by mcnee@chromium.org
, Jan 18 2018