Closed Bug 1409079 Opened 7 years ago Closed 7 years ago

stylo: re-enable web components and update tests.

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(2 files)

Bug 1409079: Re-enable web components in stylo.
(deleted), text/x-review-board-request
heycam
: review+
smaug
: review+
Details
Bug 1409079: Simplify WebComponents enabled checks.
(deleted), text/x-review-board-request
smaug
: review+
Details
They're surprisingly close to working to most extent.
Depends on: stylo-shadow
Blocks: 1409086
Blocks: stylo-shadow
No longer depends on: stylo-shadow
Tentatively marking the dependency on bug 1404789 since I haven't tested this patch without it.
Depends on: 1404789
Comment on attachment 8919005 [details] Bug 1409079: Re-enable web components in stylo. https://reviewboard.mozilla.org/r/189894/#review195220 You'll need a DOM peer's review on the test_interfaces.js change, I guess. ::: commit-message-340e9:3 (Diff revision 1) > +I can probably simplify some of the checks to only look at nsContentUtils, but > +that can be a followup, I think. Also we should remove the `stylo` annotation > +from test_interfaces.js > + > +I'll work on both tomorrow if this patch gets accepted. Some of this doesn't really belong in a commit message.
Attachment #8919005 - Flags: review?(cam) → review+
Attachment #8919005 - Flags: review?(bzbarsky)
Boris, note that the rest of the patch is already reviewed, you can just look at the interfaces.js changes if you want (feel free to review the whole patch of course).
No longer depends on: 1404789
No longer blocks: 1409086
Actually, Olli is on my timezone, and I promised to cleanup the IsWebComponentsEnabled checks...
status-firefox57: --- → wontfix
Priority: -- → P3
Comment on attachment 8919005 [details] Bug 1409079: Re-enable web components in stylo. https://reviewboard.mozilla.org/r/189894/#review195460
Attachment #8919005 - Flags: review?(bugs) → review+
Comment on attachment 8919220 [details] Bug 1409079: Simplify WebComponents enabled checks. https://reviewboard.mozilla.org/r/190138/#review195462 ::: dom/webidl/Element.webidl:242 (Diff revision 1) > > // http://w3c.github.io/webcomponents/spec/shadow/#extensions-to-element-interface > partial interface Element { > - [Throws,Func="nsDocument::IsWebComponentsEnabled"] > + [Throws,Pref="dom.webcomponents.enabled"] > ShadowRoot createShadowRoot(); > - [Func="nsDocument::IsWebComponentsEnabled"] > + [Pref="dom.webcomponents.enabled"] I'd prefer Func=nsContentUtils::IsWebComponentsEnabled since that is faster. Same with all the .webidl
Attachment #8919220 - Flags: review?(bugs) → review+
> I'd prefer Func=nsContentUtils::IsWebComponentsEnabled since that is faster. Faster along what dimension? I'd think it's slower to check (extra function call compared to just a boolean check), but a bit faster to set up (fewer AddBoolVarCache calls).
I'd assume nsContentUtils::IsWebComponentsEnabled gets inlined.
> I'd assume nsContentUtils::IsWebComponentsEnabled gets inlined. Not easily, because it's an indirect call via the function pointer stored in Prefable. Some sort of PGO/LTO might be able to do it, maybe.
ok, I learnt something new :) Feel free to use Pref.
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/a840d960ab75 Re-enable web components in stylo. r=heycam,smaug https://hg.mozilla.org/integration/autoland/rev/da7f10ba4344 Simplify WebComponents enabled checks. r=smaug
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/02dcc553e820 followup: Skip test also in release and add shell null-check. r=me
Depends on: 1414999
Depends on: 1415013
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: