New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 794083 link

Starred by 6 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Clean up style engine code after /deep/ and ::shadow were removed

Project Member Reported by hayato@chromium.org, Dec 12 2017

Issue description

/deep/ and ::shadow were removed from CSS dynamic profile (aka a CSS selector used in *.css or <style>) at M63. Now, I think there are a lot of opportunities to clean up internal style engine code because I just removed *API surfaces* around /deep/ and ::shadow.

Let's note the current status:

- We still support /deep/ and ::shadow in CSS static profile (aka a CSS selector used in querySelecto-ish APIs)

- We no longer support "::shadow" in CSS dynamic profile. '::shadow' is now an invalid token.

- /deep/ now behaves as if it were a descendant combinator in CSS dynamic profile. e.g. 'a /deep/ b' is equivalent to 'a b'. "/deep/" is still a valid token in a CSS selector. We might want to make /deep/ an invalid token in the future.

More info:
- https://bugs.chromium.org/p/chromium/issues/detail?id=489954  
- https://www.chromestatus.com/features/6750456638341120
- https://www.chromestatus.com/feature/4964279606312960  

 
Cc: futhark@chromium.org

Comment 2 by meade@chromium.org, Dec 13 2017

Labels: Hotlist-Polish
Hello Hayato,
I have several more questions:
1. You mentioned that you may want to make /deep/ an invalid token in the future. Will you please specify this future? We are developing a product for one of our clients who uses Polymer on their site. So we would not like to create the same trouble for our team again.
2. As for JS selectors, please confirm if /deep/ token is/will be supported. 
3. Presumably in a case when /deep/ is invalid, what would you suggest we use instead?

Comment 4 by kochi@chromium.org, Dec 20 2017

1. "div /deep/ span" now matches when span is a descendant of div in the
   same tree. Now '/deep/' doesn't pierce shadow tree boundary.
   Once '/deep/' becomes an invalid token, the selector does not match any
   element, at all.  So any new usage of '/deep/' is discouraged, and
   please understand the risk that it may stop working in the future.
2. JS selector = querySelector() and querySelectorAll(), and in an argument
   for these APIs, ::shadow and /deep/ are still supported in Chrome.
   But considering Shadow DOM V0 API's deprecation, you cannot assume
   this will be supported forever.
3. CSS custom properties etc.
   https://developer.mozilla.org/en-US/docs/Web/CSS/--*

Comment 5 by kochi@chromium.org, Dec 20 2017

Cc: kochi@chromium.org

Comment 6 by kochi@chromium.org, Dec 26 2017

Cc: dpa...@chromium.org
+dpapad@ some code in ui/webui still uses /deep/:

ui/webui/resources/css/i18n_process.css:* /deep/ [i18n-content]::before,
ui/webui/resources/css/i18n_process.css:* /deep/ [i18n-values*='.innerHTML:']::before {
ui/webui/resources/css/i18n_process.css:* /deep/ [i18n-processed][i18n-content]::before,
ui/webui/resources/css/i18n_process.css:* /deep/ [i18n-processed] [i18n-content]::before,
ui/webui/resources/css/i18n_process.css:* /deep/ [i18n-processed][i18n-values*='.innerHTML:']::before,
ui/webui/resources/css/i18n_process.css:* /deep/ [i18n-processed] [i18n-values*='.innerHTML:']::before {
ui/webui/resources/js/i18n_template_no_process.js:    prefixes.push('* /deep/ ')

Now /deep/ works as a normal descendant combinator as a fallback, and will stop working
that - will become an invalid token.
Could you remove these usages - or could you forward this to someone more appropriate?
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2370d808c46d420802846ff53825e39c14b6b9eb

commit 2370d808c46d420802846ff53825e39c14b6b9eb
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Wed Dec 27 12:20:20 2017

Remove unused code after /deep/ and ::shadow removal

Now /deep/ and ::shadow only should be allowed from
querySelector() and querySelectorAll().
The only exception is that some pseudo elements (e.g. -webkit-*)
are internally translated to using ::shadow.

Bug: 794083
Change-Id: I3f2ee766458b872eaebf7b2dea7a136bcc541416
Reviewed-on: https://chromium-review.googlesource.com/844012
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526218}
[modify] https://crrev.com/2370d808c46d420802846ff53825e39c14b6b9eb/third_party/WebKit/Source/core/css/SelectorChecker.cpp

Project Member

Comment 8 by sheriffbot@chromium.org, Dec 27

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/139c95620bffee2d31ea6aaf8f97f66aba6a59f4

commit 139c95620bffee2d31ea6aaf8f97f66aba6a59f4
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Jan 03 11:08:45 2019

Remove unnecessary /deep/ selectors.

In the dynamic profile, /deep/ have been translated into descendant
combinators for a while. Removing selectors from webui which should
have no effect.

Bug: 794083
Change-Id: Ic30ad81bd8ae14adf4f36b4fdc718b355dd96fb1
Reviewed-on: https://chromium-review.googlesource.com/c/1392952
Reviewed-by: Dan Beam <dbeam@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619618}
[modify] https://crrev.com/139c95620bffee2d31ea6aaf8f97f66aba6a59f4/ui/webui/resources/css/i18n_process.css

Sign in to add a comment