Bug 217635 - Elements in Shadow DOM are wrongly marked as stale by the WebDriver
Summary: Elements in Shadow DOM are wrongly marked as stale by the WebDriver
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebDriver (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Minor
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-12 14:39 PDT by linkgoron
Modified: 2020-10-22 09:01 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.97 KB, patch)
2020-10-12 14:53 PDT, linkgoron
no flags Details | Formatted Diff | Diff
Patch (4.11 KB, patch)
2020-10-13 16:01 PDT, linkgoron
no flags Details | Formatted Diff | Diff
Patch (4.11 KB, patch)
2020-10-21 16:33 PDT, linkgoron
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description linkgoron 2020-10-12 14:39:39 PDT
Elements in Shadow DOM are wrongly marked as stale by WebDriver when issuing commands. 
For example, see the shadow_dom element click tests in WPT.
Comment 1 linkgoron 2020-10-12 14:53:29 PDT
Created attachment 411160 [details]
Patch
Comment 2 linkgoron 2020-10-12 14:56:23 PDT
The above patch fixes the issue where elements are marked as stale when issuing additional commands with the element for WebDriver.

In addition, I also fixed the focus script to work for elements in shadow DOM (as otherwise the WebDriver returns: "element not interactable: Element is not focusable.)"
Comment 3 Ryosuke Niwa 2020-10-12 23:50:20 PDT
Comment on attachment 411160 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411160&action=review

> Source/WebDriver/Session.cpp:2088
> +                    "    var activeElement = doc.activeElement;"

Please use let instead of var like the rest of the function.

> Source/WebDriver/Session.cpp:2090
> +                    "       var shadowRoot = activeElement.shadowRoot;"

This will only work with open shadow roots. That's not right.

> Source/WebDriver/Session.cpp:2098
> +                    "    if (element != activeElement)"

This will do that trick:
element != element.getRootNode().activeElement
Comment 4 linkgoron 2020-10-13 16:01:03 PDT
Created attachment 411275 [details]
Patch
Comment 5 linkgoron 2020-10-13 16:04:05 PDT
Really great suggestion! I think I made the changes that you wanted. Your suggestion also made me think that the initial focus check, that makes sure that you don't refocus the same element, wasn't exactly correct, so I also changed that.
Comment 6 Radar WebKit Bug Importer 2020-10-19 14:40:25 PDT
<rdar://problem/70456975>
Comment 7 BJ Burg 2020-10-19 14:57:10 PDT
Comment on attachment 411275 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411275&action=review

r=me

Please fix typos in the Changelog and I'll set cq+.

> Source/WebDriver/ChangeLog:9
> +        instead of document.contains which doesnt work for elements in Shadow DOM

Nit: 'doesn't'
Nit: ending period.

> Source/WebKit/ChangeLog:9
> +        instead of declaring them as not interactable

Nit: ending period.
Comment 8 linkgoron 2020-10-21 16:33:01 PDT
Created attachment 412045 [details]
Patch
Comment 9 BJ Burg 2020-10-22 08:53:00 PDT
Comment on attachment 412045 [details]
Patch

r=me
Comment 10 EWS 2020-10-22 09:01:45 PDT
Committed r268867: <https://trac.webkit.org/changeset/268867>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412045 [details].