Allow creating XUL Custom Elements in HTML documents, and HTML Custom Elements in XUL Documents

RESOLVED FIXED in Firefox 63

Status

()

P2
normal
RESOLVED FIXED
8 months ago
10 days ago

People

(Reporter: bgrins, Assigned: mossop)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
Right now Custom Elements appear unaware of the namespace of the defined element, which leads to errors when attempting to load a XUL custom element in an HTML document.

For instance, this works in browser.xul (running in the browser console):

```
{
customElements.define("foo-element", class Foo extends XULElement { bar() { return 1 } });
document.createElement("foo-element").bar()
}
```

But when loading that code in a chrome HTML document, we get:

```
TypeError: Illegal constructor.
TypeError: document.createElement(...).bar is not a function
```

AFAIK the spec doesn't define behavior for cross-namespace Custom Elements. If it's possible to infer the namespace of the element in the registry based on the JS class that's being extended that would be wonderful. Alternatively, we could pass the namespace in as an option to customElements.define().
(Assignee)

Comment 1

8 months ago
The problem is that we attempt to check the binding constructor used based on the namespace of the document itself: https://searchfox.org/mozilla-central/source/dom/bindings/BindingUtils.cpp#3813

There are two things I can think of to do this:

1. Attempt to figure out the namespace of the element in a different way. At least for non built-in elements we can just look at the constructor, if it is HTMLElement then it's html of course, if it is something else we can walk the constructor's prototypes and see if it reaches XULElement. That might be a performance problem though.

2. Add a new namespace field to the custom element definition for chrome only. So you could do something like:

customElements.define("x-foo", XFoo, { namespace: "..." });

It looks like SVG would suffer the same problem so I'm wondering if SVG just isn't supported in custom elements or if there is any plan there?
Flags: needinfo?(bugs)

Updated

8 months ago
Priority: -- → P2
As of now there are no plans to support SVG.

However, there is an old proposal https://github.com/w3c/webcomponents/issues/634
Adding namespace to ElementDefinitionOptions sounds quite reasonable, if we want to do this.

But what is the plan with XUL elements? Do we want to get rid of them eventually (though, it is unclear to me how to deal with menus and tooltips and such)
Flags: needinfo?(bugs)
(Reporter)

Comment 3

8 months ago
(In reply to Olli Pettay [:smaug] from comment #2)
> But what is the plan with XUL elements? Do we want to get rid of them
> eventually (though, it is unclear to me how to deal with menus and tooltips
> and such)

Generally yes, although that happening depends on at least a couple of moving parts:

1) Get elements off of XBL
2) Enable flexbox emulation so that we always use CSS flexbox even for XUL - thus HTML and XUL elements play nicely together

At that point I believe we could start extending HTMLElement instead of XULElement for the more generic XUL elements. As you mention, we will likely continue to use XUL elements for menus and whatnot regardless.
(Assignee)

Comment 4

8 months ago
When a custom element is defined we can check whether its class is an instance
of XULElement or HTMLElement and tag the defintion with a namespace accordingly.
This allows us to know the correct namespace for the custom element when
created.
(Assignee)

Comment 5

8 months ago
Comment on attachment 8997146 [details]
Bug 1480465: Infer the namespace for custom elements at definition time by following the class hierarchy. r=smaug

It occurred to me that we don't really need to pass the namespace to customElements.define, we cat just check whether the class passed is an instance of HTMLElement or XULElement. I need to add more tests but does this approach seem reasonable?
Attachment #8997146 - Flags: feedback?(vporof)
Attachment #8997146 - Flags: feedback?(bugs)
Comment on attachment 8997146 [details]
Bug 1480465: Infer the namespace for custom elements at definition time by following the class hierarchy. r=smaug

So, maybe, but need to see microbenchmark results.
Attachment #8997146 - Flags: feedback?(bugs)
(Reporter)

Comment 7

8 months ago
Victor, I'd also be curious if this allows us to load customElements.js in chrome HTML (IOW if this properly supports the non-dash XUL CE permission even when called from an html doc). Basically, can we remove the XUL condition here https://searchfox.org/mozilla-central/rev/e52cd92858800a69b74cb97d26d9bdb960d611ca/toolkit/components/processsingleton/MainProcessSingleton.js#82 and then get access to stuff like <xul:deck /> as a CE (which is loaded through https://searchfox.org/mozilla-central/rev/e52cd92858800a69b74cb97d26d9bdb960d611ca/toolkit/content/customElements.js#119).
(Assignee)

Comment 8

8 months ago
I created a simple page that registers 400000 custom elements (Foo extends Baz extends Bar extends HTMLElement) to attempt to get an idea of timing. The numbers fluctuate so wildly that it doesn't seem possible to use simple timing to judge whether the patch makes things slower or not.

I took some profiles though and InferNamespace does show up but it's fairly cheap, 1% of total CustomElementRegistry::Define calls. Most of that cost is actually in getting the constructors for HTMLElement and XULElement (even after breaking that out of the loop). HTML is the longest so it might actually be cheaper to not do that and just assume if we never hit anything then it is a html element.

https://perfht.ml/2vA89ku

Is this helpful?
Flags: needinfo?(bugs)
What about creating new elements use new Foo() ? That is what I'm more worried, element creation time.
Flags: needinfo?(bugs)
(Assignee)

Comment 10

8 months ago
(In reply to Olli Pettay [:smaug] from comment #9)
> What about creating new elements use new Foo() ? That is what I'm more
> worried, element creation time.

How do you expect element creation time to be impacted? InferNamespace is only called at custom element registration time, element creation time shouldn't be impacted at all by this patch.
oops, I should be looking at the patch when commenting again.
But ok, then looks reasonable.
(Reporter)

Updated

8 months ago
Blocks: 1481882
Attachment #8997146 - Attachment description: Bug 1480465: Infer the namespace for custom elements at definition time by following the class hierarchy. → Bug 1480465: Infer the namespace for custom elements at definition time by following the class hierarchy. r=smaug
(Assignee)

Updated

7 months ago
Attachment #8997146 - Flags: feedback?(vporof) → review?(bugs)
Comment on attachment 8997146 [details]
Bug 1480465: Infer the namespace for custom elements at definition time by following the class hierarchy. r=smaug

Olli Pettay [:smaug] has approved the revision.
Attachment #8997146 - Flags: review+

Updated

7 months ago
Attachment #8997146 - Flags: review?(bugs)

Comment 14

7 months ago
Pushed by dtownsend@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/413c0f58374c
Infer the namespace for custom elements at definition time by following the class hierarchy. r=smaug

Comment 15

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/413c0f58374c
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee: nobody → dtownsend
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.