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

Issue 601585 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Combination of :host-context and :host doesn't work

Reported by ikobyl...@gmail.com, Apr 7 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2702.0 Safari/537.36

Steps to reproduce the problem:
Chrome 49 handles the given selector correctly, but Chrome 50 doesn't:

<style>
  :host-context(smth):host(smth) {
    ...
  }
</style>

See the example: http://codepen.io/anon/pen/jqYLxO?editors=1010

What is the expected behavior?

What went wrong?
Since Chrome 50 such selectors match nothing

Did this work before? Yes Chrome 49

Chrome version: 51.0.2702.0  Channel: canary
OS Version: OS X 10.11.3
Flash Version: Shockwave Flash 21.0 r0

Might be broken after https://codereview.chromium.org/1703893002
 
Cc: nyerramilli@chromium.org
Components: Blink>CSS
Labels: M-50 OS-Linux OS-Windows
Owner: r...@opera.com
Status: Assigned (was: Unconfirmed)
Thanks for the report.
Bisect Info:
ChangeLog URL:
-----------------
https://chromium.googlesource.com/chromium/src/+log/67f195ef19365fce612b2d84799a2c7b26c2df96..f451cdbd381c7707f644d41cc1fe30f4cc8a32a2

as mentioned in bug report, suspecting https://codereview.chromium.org/1703893002, rune@ Could you please check the above issue & help us in finding an owner it its not yours.

Good Build: 50.0.2654.0
Bad Build: 50.0.2655.0

Able to reproduce the issue on Win7, Mac OS X 10.11.3, Ubuntu 14.04 using Chrome Beta 50.0.2661.66, Dev 51.0.2700.0 and Canary 52.0.2705.0
Broken in M50.
Labels: hasbisect

Comment 3 by r...@opera.com, Apr 11 2016

That looks like mine, yes.

Comment 4 by r...@opera.com, Apr 11 2016

Status: Started (was: Assigned)
https://codereview.chromium.org/1872343002/
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 14 2016

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

commit fd7037ec2f171b5f917f90ece37ba20b4d8506ce
Author: rune <rune@opera.com>
Date: Thu Apr 14 08:42:06 2016

Allow multiple host pseudos in same compound.

As part of optimizing away :host(-context) selectors which never may
match, we also skipped selectors with multiple :host(-context) pseudos
in the same compound.

Removed assert in findBestRuleSetAndAdd as that would now be required
to traverse the whole compound again to cover everything.

BUG= 601585 

Review URL: https://codereview.chromium.org/1872343002

Cr-Commit-Position: refs/heads/master@{#387263}

[add] https://crrev.com/fd7037ec2f171b5f917f90ece37ba20b4d8506ce/third_party/WebKit/LayoutTests/fast/dom/shadow/multiple-host-pseudos-in-compound-expected.txt
[add] https://crrev.com/fd7037ec2f171b5f917f90ece37ba20b4d8506ce/third_party/WebKit/LayoutTests/fast/dom/shadow/multiple-host-pseudos-in-compound.html
[modify] https://crrev.com/fd7037ec2f171b5f917f90ece37ba20b4d8506ce/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/fd7037ec2f171b5f917f90ece37ba20b4d8506ce/third_party/WebKit/Source/core/css/RuleSet.cpp

Comment 6 by r...@opera.com, Apr 14 2016

Status: Fixed (was: Started)

Comment 7 by timloh@chromium.org, Apr 20 2016

Cc: kochi@chromium.org dpranke@chromium.org hayato@chromium.org r...@opera.com
 Issue 604633  has been merged into this issue.

Comment 8 by timloh@chromium.org, Apr 20 2016

 Issue 604939  has been merged into this issue.

Comment 9 by timloh@chromium.org, Apr 20 2016

Status: Assigned (was: Fixed)
Do we want to merge to M51?

Comment 10 by kochi@chromium.org, Apr 20 2016

and M50 as well?
It would be very nice if this fix could be merged to M50.

Comment 12 by kochi@chromium.org, Apr 20 2016

Labels: -Pri-2 Merge-Request-51 Pri-1
Let's request for Merge to M51 first.

Comment 13 by tin...@google.com, Apr 20 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)

Comment 14 by r...@opera.com, Apr 20 2016

I'll do the merge.

Comment 15 by kochi@chromium.org, Apr 20 2016

oh, I've done the moments ago for M51.
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 20 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b364f41cbcfd7b2a2d9ffafe188431df0a9edb27

commit b364f41cbcfd7b2a2d9ffafe188431df0a9edb27
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Wed Apr 20 07:41:46 2016

Allow multiple host pseudos in same compound.

As part of optimizing away :host(-context) selectors which never may
match, we also skipped selectors with multiple :host(-context) pseudos
in the same compound.

Removed assert in findBestRuleSetAndAdd as that would now be required
to traverse the whole compound again to cover everything.

BUG= 601585 

Review URL: https://codereview.chromium.org/1872343002

Cr-Commit-Position: refs/heads/master@{#387263}
(cherry picked from commit fd7037ec2f171b5f917f90ece37ba20b4d8506ce)

Review URL: https://codereview.chromium.org/1903143002 .

Cr-Commit-Position: refs/branch-heads/2704@{#143}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[add] https://crrev.com/b364f41cbcfd7b2a2d9ffafe188431df0a9edb27/third_party/WebKit/LayoutTests/fast/dom/shadow/multiple-host-pseudos-in-compound-expected.txt
[add] https://crrev.com/b364f41cbcfd7b2a2d9ffafe188431df0a9edb27/third_party/WebKit/LayoutTests/fast/dom/shadow/multiple-host-pseudos-in-compound.html
[modify] https://crrev.com/b364f41cbcfd7b2a2d9ffafe188431df0a9edb27/third_party/WebKit/Source/core/css/RuleFeature.cpp
[modify] https://crrev.com/b364f41cbcfd7b2a2d9ffafe188431df0a9edb27/third_party/WebKit/Source/core/css/RuleSet.cpp

Comment 17 by r...@opera.com, Apr 20 2016

ok. thanks.

Comment 18 by kochi@chromium.org, Apr 20 2016

Owner: kochi@chromium.org
Status: Started (was: Assigned)
I'll handle M50 merge.
Let's wait to see how it lands in M51 beta.

Comment 19 by kochi@chromium.org, Apr 22 2016

Now M51 beta (50.0.2704.22) includes the merge.
Let's see over the weekend.

Comment 20 by kochi@chromium.org, Apr 26 2016

Labels: Merge-Request-50
this time requesting merge for stable M50.

Comment 21 by tin...@google.com, Apr 26 2016

Labels: -Merge-Request-50 Merge-Review-50 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M50), manual review required.
Before we approve merge to M50, Could you please confirm whether this bug is baked/verified in Canary/beta and safe to merge? 

Comment 23 by kochi@chromium.org, Apr 27 2016

Canary since Apr.15, Beta since Apr.22, so about 4 days.
I confirmed that the change cleanly applies to stable.

I think it's safe to merge to stable.
Cc: tinazh@chromium.org
Labels: -Merge-Review-50 Merge-Approved-50
Approving merge to M50 branch 2661 based on comment #23. Please merge ASAP. Thank you.
Project Member

Comment 25 by bugdroid1@chromium.org, Apr 27 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/369818b67173d3ed4cbbc3af82fcb17a9a5528cf

commit 369818b67173d3ed4cbbc3af82fcb17a9a5528cf
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Wed Apr 27 06:11:39 2016

Allow multiple host pseudos in same compound.

As part of optimizing away :host(-context) selectors which never may
match, we also skipped selectors with multiple :host(-context) pseudos
in the same compound.

Removed assert in findBestRuleSetAndAdd as that would now be required
to traverse the whole compound again to cover everything.

BUG= 601585 

Review URL: https://codereview.chromium.org/1872343002

Cr-Commit-Position: refs/heads/master@{#387263}
(cherry picked from commit fd7037ec2f171b5f917f90ece37ba20b4d8506ce)

Review URL: https://codereview.chromium.org/1917403002 .

Cr-Commit-Position: refs/branch-heads/2661@{#636}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[add] https://crrev.com/369818b67173d3ed4cbbc3af82fcb17a9a5528cf/third_party/WebKit/LayoutTests/fast/dom/shadow/multiple-host-pseudos-in-compound-expected.txt
[add] https://crrev.com/369818b67173d3ed4cbbc3af82fcb17a9a5528cf/third_party/WebKit/LayoutTests/fast/dom/shadow/multiple-host-pseudos-in-compound.html
[modify] https://crrev.com/369818b67173d3ed4cbbc3af82fcb17a9a5528cf/third_party/WebKit/Source/core/css/RuleFeature.cpp

Comment 26 by kochi@chromium.org, Apr 27 2016

Status: Fixed (was: Started)

Sign in to add a comment