:unresolved CSS pseudo-class erroneously matches defined elements in V1 |
||||||
Issue descriptionSee minimal repro at https://jsfiddle.net/jfynw40e/2/ According to [1] in Shadow DOM v1, :unresolved is not supported, and should be replaced by :not(:defined) instead. However, when writing code that needs to work both with Shadow DOM v0 and v1 (temporarily during the migration), the following CSS rule is not adequate x-foo:unresolved, x-foo:not(:defined) { display: none; } because the :unresolved selector matches even when x-foo is defined. This is affecting issue 884030 (and Chromium Shadow DOM v1 migration in general). [1] https://github.com/Polymer/docs/issues/2297#issuecomment-326465723
,
Sep 26
,
Sep 26
I
,
Sep 26
So, :defined also should not match to defined V0 custom elements, I think.
,
Sep 27
The following table shows the current behavior: :unresolved :not(:defined) :defined Built-in false false true Not-defined true true false V0-defined false true false V1-defined true false true I propose changing this to the following table: :unresolved :not(:defined) :defined Built-in false false true Not-defined true true false V0-defined false false* true* V1-defined false* false true i.e. :unresolved and :defined won't distinguish V0 and V1.
,
Sep 27
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/65b8a51a966416f5c0b798966b6eb09b466a8075 commit 65b8a51a966416f5c0b798966b6eb09b466a8075 Author: Kent Tamura <tkent@chromium.org> Date: Thu Sep 27 03:40:15 2018 custom-elements: Do not distinguish V0 custom elements and V1 custom elements in :unresolved and :defined matching. This CL changed the behavior of :unresolved selector for V1-defined custom elements, and the behavior of :defined selector for V0-defined custom elements. The following table shows the current behavior: :unresolved :not(:defined) :defined Built-in false false true Not-defined true true false V0-defined false true false V1-defined true false true After this CL, the behavior is: :unresolved :not(:defined) :defined Built-in false false true Not-defined true true false V0-defined false false* true* V1-defined false* false true V0 custom elements specification defined ':unresolved' and V1 custom elements specification defines ':defined', but we don't need to distinguish V0-defined elements and V1-defined elements for these selectors. The current behavior makes it difficult to migrate V0 custom elements to V1 custom elements gradually. Bug: 889336 Change-Id: I48ecced042fe78314b42da4ea65baddb720e38fc Reviewed-on: https://chromium-review.googlesource.com/1248061 Reviewed-by: Hayato Ito <hayato@chromium.org> Commit-Queue: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/master@{#594582} [modify] https://crrev.com/65b8a51a966416f5c0b798966b6eb09b466a8075/third_party/WebKit/LayoutTests/custom-elements/v0-v1-interop.html [modify] https://crrev.com/65b8a51a966416f5c0b798966b6eb09b466a8075/third_party/blink/renderer/core/css/selector_checker.cc
,
Sep 27
,
Sep 27
Thank you for fixing this!
,
Sep 28
I am worried about the implications of this change for compatibility. Doesn't this make it easier for web developers to write stylesheets that use :unresolved + custom elements v1, and thus their styling only works in Chrome (even though their element works in other browsers)? It seems likely this will end up causing :unresolved to get added to specs since other browsers will need to implement it to avoid compat problems.
,
Sep 28
I'll add a deprecation message for :unresolved usage.
,
Sep 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e57a8b12a89bc6f38a09fa60c365f80e268c7c5e commit e57a8b12a89bc6f38a09fa60c365f80e268c7c5e Author: Kent Tamura <tkent@chromium.org> Date: Fri Sep 28 07:44:49 2018 Deprecate :unresolved pseudo selector. :unresolved selector is a part of Custom Elements V0, and should be removed with document.registerElement(). This is a leftover of http://crrev.com/587495. Intent to Deprecate and Remove: https://groups.google.com/a/chromium.org/d/msg/blink-dev/h-JwMiPUnuU/sl79aLoLBQAJ This CL also fixes an issue of CSSSelectorParser:: RecordUsageAndDeprecations(). It didn't show a deprecation message if the function was called with style_sheet_==nullptr, then was called with a valid style_sheet_. Change-Id: I5b1b16c722da7bc85fe24a63432e0805fbbed087 Bug: 660759, 889336 Reviewed-on: https://chromium-review.googlesource.com/1250345 Reviewed-by: Yoichi Osato <yoichio@chromium.org> Commit-Queue: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/master@{#595012} [modify] https://crrev.com/e57a8b12a89bc6f38a09fa60c365f80e268c7c5e/third_party/WebKit/LayoutTests/fast/css/invalidation/unresolved-pseudo-expected.txt [modify] https://crrev.com/e57a8b12a89bc6f38a09fa60c365f80e268c7c5e/third_party/WebKit/LayoutTests/fast/dom/custom/element-upgrade-expected.txt [modify] https://crrev.com/e57a8b12a89bc6f38a09fa60c365f80e268c7c5e/third_party/WebKit/LayoutTests/fast/dom/custom/unresolved-pseudoclass-expected.txt [modify] https://crrev.com/e57a8b12a89bc6f38a09fa60c365f80e268c7c5e/third_party/WebKit/LayoutTests/webexposed/custom-elements-expected.txt [modify] https://crrev.com/e57a8b12a89bc6f38a09fa60c365f80e268c7c5e/third_party/blink/renderer/core/css/parser/css_selector_parser.cc [modify] https://crrev.com/e57a8b12a89bc6f38a09fa60c365f80e268c7c5e/third_party/blink/renderer/core/frame/deprecation.cc
,
Sep 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/652ce3fa85fbeb51c50e3885d994350fdcf56ae8 commit 652ce3fa85fbeb51c50e3885d994350fdcf56ae8 Author: Kent Tamura <tkent@chromium.org> Date: Fri Sep 28 10:48:46 2018 custom-elements: Invalidate :unresolved and :defined correctly. We changed the coverage of :unresolved and :defined in crrev.com/594582. But the CL didn't invalidate style correctly. This CL fixes it. * node.cc: - Invalidate :unresolved on V1 custom element state change - Invalidate :defined on V0 custom element state change * Tests: Add v1 custom element + :unresolved test to existing unresolved-pseudo.html. Add :defined invalidation test. Bug: 889336 Change-Id: Id69ba3f9cb62d2805877b648762f4f94bd02e1f9 Reviewed-on: https://chromium-review.googlesource.com/1250721 Commit-Queue: Kent Tamura <tkent@chromium.org> Reviewed-by: Hayato Ito <hayato@chromium.org> Cr-Commit-Position: refs/heads/master@{#595048} [add] https://crrev.com/652ce3fa85fbeb51c50e3885d994350fdcf56ae8/third_party/WebKit/LayoutTests/fast/css/invalidation/defined-pseudo.html [modify] https://crrev.com/652ce3fa85fbeb51c50e3885d994350fdcf56ae8/third_party/WebKit/LayoutTests/fast/css/invalidation/unresolved-pseudo-expected.txt [modify] https://crrev.com/652ce3fa85fbeb51c50e3885d994350fdcf56ae8/third_party/WebKit/LayoutTests/fast/css/invalidation/unresolved-pseudo.html [modify] https://crrev.com/652ce3fa85fbeb51c50e3885d994350fdcf56ae8/third_party/blink/renderer/core/dom/node.cc |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by dpa...@chromium.org
, Sep 26