WICG / webcomponents

Web Components specifications
Other
4.38k stars 371 forks source link

Support `>>>` combinator in static profile #78

Closed hayatoito closed 6 years ago

hayatoito commented 9 years ago

Title: [Shadow]: Figure out a good replacement for /deep/ in testing scenarios (bugzilla: 28591)

Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591


comment: 0 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c0 Dimitri Glazkov wrote on 2015-05-01 17:22:53 +0000.

One thing that immediately popped up once we started talking about removing shadow-piercing combinators is that WebDriver-based tests are frequently interested in reaching into shadow trees to grab a specific element to test:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/chromedriver/test/run_py_tests.py&sq=package:chromium&q=testShadowDom*&l=877

Web Driver actually has something currently that attempts to solve the problem: http://www.w3.org/TR/webdriver/#switching-to-hosted-shadow-doms

However, the feedback I got from ChromeDriver folks is that it's way too verbose and awkward to use for the most common case (see above).

Maybe the solution is just specifying deepQuerySelector for WebDriver spec. However, I want to make sure this is not just a one-off -- seems like this could be needed in other contexts.


comment: 1 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c1 Anne wrote on 2015-05-02 07:34:03 +0000.

1) We shouldn't do features for testing. That's bad.

2) I remain convinced that in the open case we should provide a myriad of features that cross the "deep" to aid with selection, event delegation, composition, etc.


comment: 2 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c2 Elliott Sprehn wrote on 2015-05-03 00:41:03 +0000.

(In reply to Anne from comment #1)

1) We shouldn't do features for testing. That's bad.

2) I remain convinced that in the open case we should provide a myriad of features that cross the "deep" to aid with selection, event delegation, composition, etc.

+1, we should keep /deep/ in the static profile for querySelector. Before we had it authors kept rolling their own (we saw this on multiple occasions).


comment: 3 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c3 Anne wrote on 2015-05-04 06:12:37 +0000.

Note that an alternative is that we introduce .deepQuery() or some such.


comment: 4 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c4 Elliott Sprehn wrote on 2015-05-04 06:21:02 +0000.

(In reply to Anne from comment #3)

Note that an alternative is that we introduce .deepQuery() or some such.

deepQuery is not enough, you don't want to match a descendant selector across a ShadowRoot boundary since ".a .b" means something really different. You'd still need a special combinator to signal where the scope crossing should be in the selector expression.

ex. .panel .image

All images inside panels contained in a single scope.

.panel /deep/ .image

All images anywhere below a panel, even if they're inside a nested widget.

This is important because it maintains the "don't accidentally cross a boundary" principle.

We need something like ::shadow as well.


comment: 5 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c5 Tab Atkins Jr. wrote on 2015-05-05 00:12:16 +0000.

(In reply to Elliott Sprehn from comment #4)

(In reply to Anne from comment #3)

Note that an alternative is that we introduce .deepQuery() or some such.

deepQuery is not enough, you don't want to match a descendant selector across a ShadowRoot boundary since ".a .b" means something really different. You'd still need a special combinator to signal where the scope crossing should be in the selector expression.

ex. .panel .image

All images inside panels contained in a single scope.

.panel /deep/ .image

All images anywhere below a panel, even if they're inside a nested widget.

This is important because it maintains the "don't accidentally cross a boundary" principle.

Yeah, trying to move the shadow-crossing quality to the core of the method doesn't work. It's much less flexible, as you note, and doesn't compose with anything else similar. The correct approach is to just embrace the "static profile" of selectors http://dev.w3.org/csswg/selectors/#static-profile and leave /deep/ there. (Or >>>, as it's now called.)


comment: 6 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c6 Hayato Ito wrote on 2015-05-07 08:43:56 +0000.

(In reply to Tab Atkins Jr. from comment #5)

(In reply to Elliott Sprehn from comment #4)

(In reply to Anne from comment #3)

Note that an alternative is that we introduce .deepQuery() or some such.

deepQuery is not enough, you don't want to match a descendant selector across a ShadowRoot boundary since ".a .b" means something really different. You'd still need a special combinator to signal where the scope crossing should be in the selector expression.

ex. .panel .image

All images inside panels contained in a single scope.

.panel /deep/ .image

All images anywhere below a panel, even if they're inside a nested widget.

This is important because it maintains the "don't accidentally cross a boundary" principle.

Yeah, trying to move the shadow-crossing quality to the core of the method doesn't work. It's much less flexible, as you note, and doesn't compose with anything else similar. The correct approach is to just embrace the "static profile" of selectors http://dev.w3.org/csswg/selectors/#static-profile and leave /deep/ there. (Or >>>, as it's now called.)

Is there any existing clients who use static-profile? Does it mean '/deep/' can be used in particular APIs?


comment: 7 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c7 Tab Atkins Jr. wrote on 2015-05-07 15:55:04 +0000.

(In reply to Hayato Ito from comment #6)

Is there any existing clients who use static-profile? Does it mean '/deep/' can be used in particular APIs?

It's for querySelector()/etc.


comment: 8 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c8 Hayato Ito wrote on 2015-05-08 02:25:11 +0000.

(In reply to Tab Atkins Jr. from comment #7)

(In reply to Hayato Ito from comment #6)

Is there any existing clients who use static-profile? Does it mean '/deep/' can be used in particular APIs?

It's for querySelector()/etc.

Thanks.

Can everyone agree that '/deep/' is okay to be used in querySelector()?

I think we are assuming that adding something to static profile is zero-overhead to the performance of dynamic profile.


comment: 9 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28591#c9 Tab Atkins Jr. wrote on 2015-05-08 17:29:16 +0000.

(In reply to Hayato Ito from comment #8)

(In reply to Tab Atkins Jr. from comment #7)

(In reply to Hayato Ito from comment #6)

Is there any existing clients who use static-profile? Does it mean '/deep/' can be used in particular APIs?

It's for querySelector()/etc.

Thanks.

Can everyone agree that '/deep/' is okay to be used in querySelector()?

I think we are assuming that adding something to static profile is zero-overhead to the performance of dynamic profile.

Correct. At worst, it's a check during grammar verification, to note that this isn't valid in the current context and so the selector should be considered grammar-violating.

robdodson commented 7 years ago

Even for testing, the need for querySelectorAll never came up because we could just use JS functions to find the right element to do the necessary white box testing.

Can you give me an example? The most compelling use case I've heard so far for needing a shadow piercing querySelector is for testing. Did you instead implement your own tree walking function?

rniwa commented 7 years ago

Even for testing, the need for querySelectorAll never came up because we could just use JS functions to find the right element to do the necessary white box testing.

Can you give me an example? The most compelling use case I've heard so far for needing a shadow piercing querySelector is for testing. Did you instead implement your own tree walking function?

Basically, we decided not to write tests that pierce across shadow boundaries. We wrote unit tests for each component where we reach out into each shadow tree and check its state, and once we've done that, we never reached into shadow trees in our integration tests. This is how builtin HTML elements work by the way. As a web developer, you never test internal implementation details of builtin HTML elements. The best you can do is to check its publicly exposed states, and that's exactly what we do with our components, and I'd say it's working fairly well for us.

ChadKillingsworth commented 7 years ago

Basically, we decided not to write tests that pierce across shadow boundaries.

That falls apart when you have interactive elements in shadow roots (like <button>). You end up writing public accessors just to test the component.

Having worked with this a lot (and being the author of a very popular gist on this subject), I don't want the shadow piercing combinator. What I do want is a selector that only works for testing that selects from the current element's shadow root:

document.querySelector('custom-element => button');
// Must be currently written as:
document.querySelector('custom-element').shadowRoot.querySelector('button');

However most of this could be easily overcome by syntactic sugar or special methods in testing frameworks.

equinusocio commented 7 years ago

What about the polymer way? this.shadowRoot.querySelector()

Il giorno 08 ott 2017, alle ore 22:11, Chad Killingsworth notifications@github.com ha scritto:

Basically, we decided not to write tests that pierce across shadow boundaries.

That falls apart when you have interactive elements in shadow roots (like

rniwa commented 7 years ago

Basically, we decided not to write tests that pierce across shadow boundaries.

That falls apart when you have interactive elements in shadow roots (like

I don't follow. For testing interactive elements, we do white box testing by exposing its shadow tree. There's no need for a component that uses another interactive components to reach into its shadow tree.

Having worked with this a lot (and being the author of a very popular gist on this subject), I don't want the shadow piercing combinator. What I do want is a selector that only works for testing that selects from the current element's shadow root:

document.querySelector('custom-element => button');
// Must be currently written as:
document.querySelector('custom-element').shadowRoot.querySelector('button');

That should already work. ShadowRoot.prototype.querySelector and ShadowRoot.prototype.querySelectorAll already exist.

ChadKillingsworth commented 7 years ago

There's no need for a component that uses another interactive components to reach into its shadow tree.

I disagree. For full integration tests, I need to click and interact with elements just like the user would. I frequently have form elements, anchors and other interactive elements inside a shadow tree. The alternative is using selenium to click on specific offsets in the viewport - and that is extremely fragile.

That should already work. ShadowRoot.prototype.querySelector and ShadowRoot.prototype.querySelectorAll already exist.

It does - but it's very verbose and difficult to work with. Authors want the convenience that the shadow piercing combinator provided. This problem really only exists because selenium locates elements based on query selectors. It's cumbersome to use shadow dom in that environment.

rniwa commented 7 years ago

There's no need for a component that uses another interactive components to reach into its shadow tree.

I disagree. For full integration tests, I need to click and interact with elements just like the user would. I frequently have form elements, anchors and other interactive elements inside a shadow tree. The alternative is using selenium to click on specific offsets in the viewport - and that is extremely fragile.

But you wouldn't do this with builtin elements. You wouldn't reach into video element's controls and start messing with their buttons because you trust that video element is implemented & tested correctly on its isolation.

That should already work. ShadowRoot.prototype.querySelector and ShadowRoot.prototype.querySelectorAll already exist.

It does - but it's very verbose and difficult to work with. Authors want the convenience that the shadow piercing combinator provided. This problem really only exists because selenium locates elements based on query selectors. It's cumbersome to use shadow dom in that environment.

Since what you want in your use case can be easily built as a shim on top of the existing capability, I still don't see why this needs to be implemented in the browser.

ChadKillingsworth commented 7 years ago

But you wouldn't do this with builtin elements. You wouldn't reach into video element's controls and start messing with their buttons because you trust that video element is implemented & tested correctly on its isolation.

True - but again that doesn't scale up to the way web components are used today. This is due to two base reasons:

  1. Custom elements cannot properly participate in a form. Form elements inside the component have to be tested with elements outside of the component.
  2. As you combine base elements into distinct sets, the parent element becomes an app and testing requires interacting with the children in integration tests.
<my-app>
  #shadowRoot
    <form>
      <name-fields>
        #shadowRoot
          <input type="text" name="fullname">
      </name-fields>
      <form-submission></form-submission>
    <form>
</my-app>

Take the above example. How do you test the form submission works? This is of course a contrived example, but the answer shouldn't be "don't do that". Having distinct grouping of input fields is handy to share between several forms.

As you continue to compose custom elements together, how else do your properly test the combined total? How do you test <my-app> in a final integration test in a browser?

rniwa commented 7 years ago

The form submission issue is tracked by https://github.com/w3c/webcomponents/issues/187, which we're intending to discuss at W3C TPAC this year.

As you continue to compose custom elements together, how else do your properly test the combined total? How do you test <my-app> in a final integration test in a browser?

Even in that scenario, piercing across shadow boundaries isn't really a good way to write tests. When the implementation details of each component changes, you'd be forced to update all those integration tests.

A better way to accomplish this would be relying on public APIs of each component to test, and make sure each component has a separate unit tests for public API updating its internal states correctly. If there isn't adequate API support for writing whatever tests, then you can add test/debug only API which exposes necessary states/information. We do this in WebKit's C++ code by exposing extra JS objects which lets tests expose & manipulate internal states of WebKit even though doing so in production/user environment won't be okay due to security concerns.

ChadKillingsworth commented 7 years ago

We do this in WebKit's C++ code by exposing extra JS objects which lets tests expose & manipulate internal states of WebKit even though doing so in production/user environment won't be okay due to security concerns.

This is exactly why a special selector would be nice for selenium only. I don't think adding it in general to the static profile is warranted at all and would just encourage bad things.

rniwa commented 7 years ago

We do this in WebKit's C++ code by exposing extra JS objects which lets tests expose & manipulate internal states of WebKit even though doing so in production/user environment won't be okay due to security concerns.

This is exactly why a special selector would be nice for selenium only. I don't think adding it in general to the static profile is warranted at all and would just encourage bad things.

Okay. That more or less matches our expectation. Does something like the API I proposed in https://github.com/w3c/webcomponents/issues/78#issuecomment-265601511 work for you? Instead of having an explicit >>>, this API just finds any element which matches a given selector in any shadow tree.

zhaoz commented 7 years ago

The whole point of shadow DOM API is to provide encapsulation. If your app needs to constantly break that encapsulation, then it's probably better not to use shadow DOM API in the first place.

For YouTube, the benefit of shadow DOM API is encapsulation of styling as a default. There are still benefits for global styling and the ability to select elements through shadow boundaries. That said, focusing on the dynamic profile, I do agree that this is mostly useful for testing and debugging. This is the majority (and possibly only) use case that YouTube has.

We’re experimenting with collectMatchingElementsInFlatTree (implemented in https://trac.webkit.org/changeset/208878) which finds all elements that match a given selector in each tree instead of an explicit >>> to cross a shadow boundary

Is collectMatchingElementsInFlatTree on the element prototype? E.g. allowing for this to work on a subtree? I want to be able to do something like someDiv.collectMatchingElementsInFlatTree().

Also, does this API select for trees within a shadow root? or does it only match an element in isolation? Like, are queries like #main x-foo valid if both x-foo and #main were in the same shadowRoot?

rniwa commented 7 years ago

Is collectMatchingElementsInFlatTree on the element prototype? E.g. allowing for this to work on a subtree? I want to be able to do something like someDiv.collectMatchingElementsInFlatTree().

Our API currently hangs off of the global object and you have to pass in the someDiv but if we're standardizing it, then we'd most certainly add it on an element just like what you wrote.

Also, does this API select for trees within a shadow root? or does it only match an element in isolation? Like, are queries like #main x-foo valid if both x-foo and #main were in the same shadowRoot?

#main x-foo to apply an element only if both x-foo and #main are in the same tree.

tabatkins commented 7 years ago

Oh, wow, that's a much more significant restriction in power than I thought you were talking about.

WebAppsWG commented 7 years ago

We have a flattened tree querySelectorAll implementation in axe-core that we could abstract out into a separate module if you would find it useful

https://github.com/dequelabs/axe-core

This implementation does in fact select across shadow root boundaries but has other implications in that it will not select elements in the light DOM that are assigned into slots using just a light DOM selector, you have to use a selector that resolves into the slot

--Dylan

On Tue, Oct 10, 2017 at 1:51 PM, zhaoz notifications@github.com wrote:

The whole point of shadow DOM API is to provide encapsulation. If your app needs to constantly break that encapsulation, then it's probably better not to use shadow DOM API in the first place.

For YouTube, the benefit of shadow DOM API is encapsulation of styling as a default. There are still benefits for global styling and the ability to select elements through shadow boundaries. That said, focusing on the dynamic profile, I do agree that this is mostly useful for testing and debugging. This is the majority (and possibly only) use case that YouTube has.

We’re experimenting with collectMatchingElementsInFlatTree (implemented in https://trac.webkit.org/changeset/208878) which finds all elements that match a given selector in each tree instead of an explicit >>> to cross a shadow boundary

Is collectMatchingElementsInFlatTree on the element prototype? E.g. allowing for this to work on a subtree? I want to be able to do something like someDiv.collectMatchingElementsInFlatTree().

Also, does this API select for trees within a shadow root? or does it only match an element in isolation? Like, are queries like #main x-foo valid if both x-foo and #main were in the same shadowRoot?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/w3c/webcomponents/issues/78#issuecomment-335554663, or mute the thread https://github.com/notifications/unsubscribe-auth/AJeKsRO9uv4d-pdodr1UiELSoav2fAiSks5sq66ggaJpZM4EoeE4 .

-- Download the aXe browser extension for free:

Firefox: https://addons.mozilla.org/en-US/firefox/addon/axe-devtools Chrome: https://chrome.google.com/webstore/detail/axe/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US

Life is ten percent what happens to you and ninety percent how you respond to it. - Lou Holtz

rniwa commented 6 years ago

Again, we're not interested in implementing >>> or /shadow/ in the static profile as currently proposed.

annevk commented 6 years ago

I agree and have said as much upstream in https://github.com/w3c/csswg-drafts/issues/640#issuecomment-367293984. I suggest that we continue the discussion there as this affects a document maintained by the CSS WG. I'll add that document to https://github.com/w3c/webcomponents/blob/gh-pages/README.md so it's easier to find from this repository going forward.

ericlaw1979 commented 3 years ago

As of Chrome 89.0.4357, using /deep/ in querySelectorAll() now throws an exception. I can't readily tell from this thread whether a replacement was ever created.

(My scenario was a browser extension called moarTLS: I want to scan every in the page, whether in a shadow or not, to determine whether its href points to HTTPS or HTTP.)

rniwa commented 3 years ago

As of Chrome 89.0.4357, using /deep/ in querySelectorAll() now throws an exception. I can't readily tell from this thread whether a replacement was ever created.

There is no cross-browser / standard replacement.

mfreed7 commented 3 years ago

As of Chrome 89.0.4357, using /deep/ in querySelectorAll() now throws an exception. I can't readily tell from this thread whether a replacement was ever created.

(My scenario was a browser extension called moarTLS: I want to scan every in the page, whether in a shadow or not, to determine whether its href points to HTTPS or HTTP.)

Just to confirm, Shadow DOM v0 and Custom Elements v0 have both been removed from Chromium in v89+. And that includes the (long deprecated) /deep/ combinator. Using any combinator with a '/' now throws an exception, as it always has in Safari and Firefox. And as @rniwa said, there's no replacement for /deep/ in Shadow DOM v1, sorry.

SebastianZ commented 3 years ago

Well, there is no direct replacement for /deep/ or >>>, but the ::part() pseudo-element lets you target elements within the Shadow DOM, which were explicitly exposed via the part attribute.

Sebastian

LarsDenBakker commented 3 years ago

There are JS based userland solutions for this: https://github.com/Georgegriff/query-selector-shadow-dom

Similar approaches are also implemented in testing tools, like puppeteer and webdriver IO.