It has recently removed from the HTML spec[1], and no other vendors plan to implement this[2]. This would allow make more performant style scoping for shadow trees (using the shadow root pointer instead of the set of flags we now have in the element), recover two BoolFlags for nsINode, and ease the implementation details for stylo too. [1]: https://github.com/whatwg/html/issues/552 [2]: https://lists.w3.org/Archives/Public/www-style/2016Jun/0010.html

What do you think of this Cameron? Happy to give it a shot.

To me, Web Components seems kind of heavyweight for restricting styles to a subtree, so I'm not sure I buy the arguments in the issue that authors should just use those. But I'm sympathetic to the argument that nobody else wants to implement <style scoped>, so I lean towards removing support for it. In terms of blocking stylo work, I think we can just ignore the scoped="" attribute when in a document that IsStyledByServo(). If it becomes critical to recover some nsINode flags, we can prioritise this. But I'll defer to David for whether we should remove the feature.

I'd probably prefer to wait a bit longer to see how things shake out and get a bit more time for feedback on the spec's removal decision.

FYI web-platform-tests was updated in https://github.com/w3c/web-platform-tests/pull/3685

I would wait before removing this. I assume once shadow DOM is implemented everywhere and the styling it requires is supported as well, having this in all the browsers might not be so hard, as far as I understand.

This just came to my attention, since Snooze Tabs uses it when injecting its bar at the top of the page, and I was looking into a way to do the same thing. Added dev-doc-needed so we know to watch this if anything happens with it.

(In reply to Eric Shepherd [:sheppy] from comment #6) > This just came to my attention, since Snooze Tabs uses it when injecting its > bar at the top of the page, and I was looking into a way to do the same > thing. Does this mean this bug should be flagged for addon-compat as well?

Are we leaning to remove the support of <style scope>? Otherwise, we'll need to implement it for stylo in bug 1345702.

Over to dbaron/heycam for a call on this one.

(Also, TY: Can you estimate how much work this is, now that we have the relevant machinery for XBL stylesheets?)

(Having the answer to comment 10 would be useful for answering comment 9.)

Sorry for the terrible lag. Per discussion with heycam in person, I don't feel we can reuse the machinery for XBL stylesheets to implement <style scoped> easily. XBL stylesheets are processed in nsXBLPrototypeResources, and be queried in selector matching and cascading if the element or its binding parent has any bindings, whereas <style scoped> stylesheets probably need to be added to the main style set of the document during parsing, so we'll need to reinvent something on servo to extract the correct rules. I feel this is trickier than implementing XBL stylesheets. I took 4~5 weeks to implement XBL stylesheets, so this might takes longer for me (after I sort out all the remaining follow-ups of XBL stuff).

I think the biggest argument at this point for removing it, regardless of the complexity (which I also think it's high), is that no other vendor plans to implement this.

Ok, that sounds to me like we should remove it. dbaron?

OK, I guess I'm OK with removing it, then. (We may need to reconsider readding it in the future if we or others can talk other browsers into adding it.)

In the meeting today we decided that we should pref it off on beta 55 to give us maximum lead time to determine web compat issues. Cameron is going to take this.

Turns out we don't have a pref for <style scoped> itself, only for the :scope rule. I'll try to make a minimal patch that introduces a pref and disables <style scoped> (so that if we do have to re-enable it later in the Beta cycle, that could be done just with a pref flip).

And a try run just with the pref being introduced (and set to true): https://treeherder.mozilla.org/#/jobs?repo=try&revision=69722022f04bb800eeb8ece834df674b5310e49e&group_state=expanded

Comment on attachment 8879772 [details] Bug 1291515 - Part 1: Introduce a pref for <style scoped>. https://reviewboard.mozilla.org/r/151138/#review157386 r=dbaron with the following two things ::: commit-message-416c3:3 (Diff revision 1) > +Bug 1291515 - Part 1: Introduce a pref for <style scoped>. r?dbaron > + > +MozReview-Commit-ID: 1J9IvPrC0xh I think you also need to change the function IsScopedStyleElement in nsStyleLinkElement.cpp. ::: modules/libpref/init/all.js:2850 (Diff revision 1) > // Are "-webkit-{min|max}-device-pixel-ratio" media queries supported? > // (Note: this pref has no effect if the master 'layout.css.prefixes.webkit' > // pref is set to false.) > pref("layout.css.prefixes.device-pixel-ratio-webkit", false); > > +// Is support for <style scoped> enabled in content documents? Please document also that this disables exposing the DOM APIs in both content and chrome.

Comment on attachment 8879773 [details] Bug 1291515 - Part 2: Disable pref for <style scoped>. https://reviewboard.mozilla.org/r/151140/#review157388

Comment on attachment 8879772 [details] Bug 1291515 - Part 1: Introduce a pref for <style scoped>. https://reviewboard.mozilla.org/r/151138/#review157386 > I think you also need to change the function IsScopedStyleElement in nsStyleLinkElement.cpp. I think this is OK, since we only ever call it when we are updating a style sheet that we previously determined was a scoped style sheet. But it's probably safer to update it, in case we call it elsewhere later.

Part 2 needs DOM peer review.

Comment on attachment 8879772 [details] Bug 1291515 - Part 1: Introduce a pref for <style scoped>. https://reviewboard.mozilla.org/r/151138/#review157484 rs=me on the webidl bits. Thanks for taking care of this.

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4e8b918dc410 Part 1: Introduce a pref for <style scoped>. r=bholley,dbaron https://hg.mozilla.org/integration/autoland/rev/4fac24cc0437 Part 2: Disable pref for <style scoped>. r=dbaron

Comment on attachment 8879772 [details] Bug 1291515 - Part 1: Introduce a pref for <style scoped>. (This applies to part 2, too.) Approval Request Comment [Feature/Bug causing the regression]: - [User impact if declined]: We'll have less time to determine whether there are Web compatibility issues with disabling <style scoped> in time for Stylo to ship. [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: no, just landed on autoland [Needs manual test from QE? If yes, steps to reproduce]: none [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: not very [Why is the change risky/not risky?]: The change is reasonable small. It introduces a pref, and checks it in a few places. [String changes made/needed]: none

Backed out for causing reftest assertions like https://treeherder.mozilla.org/logviewer.html#?job_id=109787853&repo=autoland https://hg.mozilla.org/integration/autoland/rev/d2c7928ea955

The failures are probably due to the reftests in e.g. layout/reftests/css-display/ that set the pref on temporarily, but by the time the document is deleted, the pref is off again.

Comment on attachment 8879772 [details] Bug 1291515 - Part 1: Introduce a pref for <style scoped>. Can you take another look at this? I've added state to nsIDocument to record the decision about whether scoped styles are enabled for the document or not. (That seemed safer than adjusting the logic in nsStyleLinkElement/HTMLStyleElement to support enabling and disabling while a document is loaded.)

Comment on attachment 8879772 [details] Bug 1291515 - Part 1: Introduce a pref for <style scoped>. https://reviewboard.mozilla.org/r/151138/#review157500

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/90018589717e Part 1: Introduce a pref for <style scoped>. r=bholley,dbaron https://hg.mozilla.org/integration/autoland/rev/9ed72cb74889 Part 2: Disable pref for <style scoped>. r=dbaron

https://hg.mozilla.org/mozilla-central/rev/90018589717e https://hg.mozilla.org/mozilla-central/rev/9ed72cb74889

Can we find out about webcompat issues some other way? E.g. with telemetry or shipping something like this to a subset of our release population via a system add-on or shield? Although I guess we can always flip the pref back if it turns out to cause issues in beta in the next month.

Rather than whether there is a significant number of uses of it (which I think is a question Telemetry is well placed to answer), I think we want to know whether there is significant breakage from disabling <style scoped>. So I feel like getting that through bug reports is going to give us a better indication of whether we need to scramble to implement <style scoped> in Stylo. Getting this information as soon as we can is important. And, as you say, flipping the pref back on again is an easy thing to do. So I'd like to confirm the beta uplift request.

No other browsers support <style scoped> so the risk should be low. I'll post a site compatibility note anyway though.

Comment on attachment 8879772 [details] Bug 1291515 - Part 1: Introduce a pref for <style scoped>. alright, let's get this in 55.0b7

Comment on attachment 8879773 [details] Bug 1291515 - Part 2: Disable pref for <style scoped>. [Triage Comment]

Thanks Julien. https://treeherder.mozilla.org/#/jobs?repo=try&revision=56dd20f01a2766bfd3d815d3d70e663f0af6e670

https://hg.mozilla.org/releases/mozilla-beta/rev/1de049d9df37 https://hg.mozilla.org/releases/mozilla-beta/rev/d55331346629

setting flags :)

(In reply to Cameron McCormack (:heycam) from comment #30) > [Is this code covered by automated tests?]: yes > [Has the fix been verified in Nightly?]: no, just landed on autoland > [Needs manual test from QE? If yes, steps to reproduce]: none Setting qe-verify- based on Cameron's assessment on manual testing needs and the fact that this fix has automated coverage.

This breaks the reader mode button on Android.

I've had a go at documenting this; see the compat tables on: https://developer.mozilla.org/en-US/docs/Web/API/HTMLStyleElement/scoped#Browser_compatibility https://developer.mozilla.org/en-US/docs/Web/HTML/Element/style#Browser_compatibility and the note on the Fx56 rel notes: https://developer.mozilla.org/en-US/Firefox/Releases/56#HTML_2 Let me know if this looks OK. Thanks!

Those docs look good, thanks!

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2017/scoped-stylesheets-are-no-longer-supported/

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #50) > I've had a go at documenting this; see the compat tables on: > > https://developer.mozilla.org/en-US/docs/Web/API/HTMLStyleElement/ > scoped#Browser_compatibility > https://developer.mozilla.org/en-US/docs/Web/HTML/Element/ > style#Browser_compatibility > > and the note on the Fx56 rel notes: > > https://developer.mozilla.org/en-US/Firefox/Releases/56#HTML_2 > > Let me know if this looks OK. Thanks! This change was uplifted to fx55 (comment 46), shouldn't mdn reflect that?

(In reply to Julien Cristau [:jcristau] from comment #53) > (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #50) > > I've had a go at documenting this; see the compat tables on: > > > > https://developer.mozilla.org/en-US/docs/Web/API/HTMLStyleElement/ > > scoped#Browser_compatibility > > https://developer.mozilla.org/en-US/docs/Web/HTML/Element/ > > style#Browser_compatibility > > > > and the note on the Fx56 rel notes: > > > > https://developer.mozilla.org/en-US/Firefox/Releases/56#HTML_2 > > > > Let me know if this looks OK. Thanks! > > This change was uplifted to fx55 (comment 46), shouldn't mdn reflect that? Ooops, I missed this detail - thanks for pointing it out. I've updated the docs to state Fx 55 for this change.