Closed Bug 1373155 Opened 3 years ago Closed 3 years ago

stylo: disable Web Components in Servo-styled documents

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Seems easier to just force Web Components to be disabled in Servo-styled documents than to fix the crashes and get valid (but incorrect) styles.
Blocks: 1371194
Blocks: 1370800
Comment on attachment 8878020 [details]
Bug 1373155 - Disable Web Components in Servo-styled documents.

Overholt said this patch was generally ok but needs some tweaks, and asked that I delegate review to wchen.
Attachment #8878020 - Flags: review?(bobbyholley) → review?(wchen)
Comment on attachment 8878020 [details]
Bug 1373155 - Disable Web Components in Servo-styled documents.

https://reviewboard.mozilla.org/r/149434/#review154154

The checks look good to me. For the shadow DOM tests, why are we now failing instead of skipping? The tests don't look very meaninful when stylo is enabled since we disable shadow DOM.

::: dom/base/nsDocument.cpp:6058
(Diff revision 1)
> -  // Check for the webcomponents permission. See Bug 1181555.
>    JSAutoCompartment ac(aCx, obj);
>    JS::Rooted<JSObject*> global(aCx, JS_GetGlobalForObject(aCx, obj));
>    nsCOMPtr<nsPIDOMWindowInner> window =
>      do_QueryInterface(nsJSUtils::GetStaticScriptGlobal(global));
> +  nsIDocument* doc = window ? window->GetExtantDoc() : nullptr;

Lets move this line to just before we use |doc|
Attachment #8878020 - Flags: review?(wchen) → review+
Comment on attachment 8878020 [details]
Bug 1373155 - Disable Web Components in Servo-styled documents.

https://reviewboard.mozilla.org/r/149434/#review154154

I thought it might be good to be notified when these tests do start passing, once we add shadow DOM support to stylo later down the track.  (It's easy to forget to re-enable skipped tests.)
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0e13adff6e7e
Disable Web Components in Servo-styled documents. r=wchen
Assignee: nobody → cam
https://hg.mozilla.org/mozilla-central/rev/0e13adff6e7e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
My super minor nit here was just that we are referring to just (AFAICT with a quick skim) Shadow DOM as "Web Components" and I'd rather we were explicit about Shadow DOM vs. Custom Elements.
Flags: needinfo?(cam)
Thanks for pointing that out.  If there are specific renamings of those nsDocument/nsContentUtils methods or the pref that you'd like, please file a bug and I can take a look.
Flags: needinfo?(cam)
You need to log in before you can comment on or make changes to this bug.