1499603 - shadowDOM will apply the style that has been removed

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[709922234]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

1. open https://codepen.io/mantou132/pen/JmpYbL
2. waiting for 2s



Actual results:

Show red text



Expected results:

Show useragent sheetstyle color text

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This is an invalidation issue, we're not invalidating the host element's style.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Hmm, that should work though: https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/layout/style/ServoStyleSet.cpp#203

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1499603 - Clear host rules from clear_cascade_data.

While at it, also measure them for about:memory.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0442e4d321bb
Clear host rules from clear_cascade_data. r=heycam

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9017845 [details]
Bug 1499603 - Clear host rules from clear_cascade_data.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 992245

User impact if declined: Potentially wrong styling in pages that use Shadow DOM's :host selector in combination with some dynamic changes.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Just adding a clear() call that was overlooked when implementing the feature.

Test took more to write than the fix :)

String changes made/needed:

Comment 6

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13570 for changes under testing/web-platform/tests

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9017845 [details]
Bug 1499603 - Clear host rules from clear_cascade_data.

63 is on release now.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: 

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): 

String changes made/needed:

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/0442e4d321bb

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 11

[Pascal Chevrel:pascalc]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
> Comment on attachment 9017845 [details]
> Bug 1499603 - Clear host rules from clear_cascade_data.
> 
> [Beta/Release Uplift Approval Request]
> 
> Feature/Bug causing the regression: Bug 992245
> 
> User impact if declined: Potentially wrong styling in pages that use Shadow
> DOM's :host selector in combination with some dynamic changes.
> 

Emilio, is that a showstopper for Shadow DOM in 63? If this patch causes a regression, would it be limited to Shadow DOM?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

(In reply to Pascal Chevrel:pascalc from comment #11)
> (In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
> > Comment on attachment 9017845 [details]
> > Bug 1499603 - Clear host rules from clear_cascade_data.
> > 
> > [Beta/Release Uplift Approval Request]
> > 
> > Feature/Bug causing the regression: Bug 992245
> > 
> > User impact if declined: Potentially wrong styling in pages that use Shadow
> > DOM's :host selector in combination with some dynamic changes.
> > 
> 
> Emilio, is that a showstopper for Shadow DOM in 63?

Probably not a showstopper, no. But it'd be somewhat sad to ship this bug, since it's impossible to work around by an author.

> If this patch causes a regression, would it be limited to Shadow DOM?

It would be limited to Shadow DOM, yeah, the patch only touches shadow-dom-specific fields.

That being said I'm pretty sure it won't cause any regression, the patch is as simple as it can get. It just does the same we do for slotted_rules right above the code I added.

Comment 13

[Pascal Chevrel:pascalc]

Comment on attachment 9017845 [details]
Bug 1499603 - Clear host rules from clear_cascade_data.

Approved for rc2 as the patch is minimal, only affects Shadow DOM which is a new 63 feature that will likely get attention from the webdev community at 63 launch, has tests and I verified the fix works in Nightly. Thanks

Comment 14

[Pascal Chevrel:pascalc]

Setting the qe-verify+ flag to get this verified on RC2 with the testcase in comment #0.

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-release/rev/f95041f37c6f

Comment 16

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

I have reproduced the issue mentioned in Description using the affected Nightly 2018-10-16 build.

This issue is verified fixed using Firefox 63.0(20181018182531)and Nightly 64.0a1(20181019100103) on Windows 10 64bit, macOS 10.13 and Ubuntu 16.04 32bit.