Closed Bug 1499603 Opened 1 year ago Closed 1 year ago

shadowDOM will apply the style that has been removed

Categories

(Core :: CSS Parsing and Computation, defect)

64 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- disabled
firefox63 --- verified
firefox64 --- verified

People

(Reporter: 709922234, Assigned: emilio)

References

()

Details

Attachments

(1 file)

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
Component: Untriaged → DOM
Product: Firefox → Core
Assignee: nobody → emilio
Status: UNCONFIRMED → NEW
Ever confirmed: true
This is an invalidation issue, we're not invalidating the host element's style.
Component: DOM → CSS Parsing and Computation
While at it, also measure them for about:memory.
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/0442e4d321bb
Clear host rules from clear_cascade_data. r=heycam
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:
Attachment #9017845 - Flags: approval-mozilla-beta?
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13570 for changes under testing/web-platform/tests
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:
Attachment #9017845 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
https://hg.mozilla.org/mozilla-central/rev/0442e4d321bb
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Upstream PR merged
(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?
Flags: needinfo?(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.
Flags: needinfo?(emilio)
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
Attachment #9017845 - Flags: approval-mozilla-release? → approval-mozilla-release+
Setting the qe-verify+ flag to get this verified on RC2 with the testcase in comment #0.
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.