Closed
Bug 1480465
Opened 6 years ago
Closed 6 years ago
Allow creating XUL Custom Elements in HTML documents, and HTML Custom Elements in XUL Documents
Categories
(Core :: DOM: Core & HTML, task, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: bgrins, Assigned: mossop)
References
Details
Attachments
(1 file)
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•6 years 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•6 years ago
|
Priority: -- → P2
Comment 2•6 years ago
|
||
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•6 years 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•6 years 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•6 years 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 6•6 years ago
|
||
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•6 years 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•6 years 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)
Comment 9•6 years ago
|
||
What about creating new elements use new Foo() ? That is what I'm more worried, element creation time.
Flags: needinfo?(bugs)
Assignee | ||
Comment 10•6 years 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.
Comment 11•6 years ago
|
||
oops, I should be looking at the patch when commenting again.
Comment 12•6 years ago
|
||
But ok, then looks reasonable.
Updated•6 years ago
|
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•6 years ago
|
Attachment #8997146 -
Flags: feedback?(vporof) → review?(bugs)
Comment 13•6 years ago
|
||
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•6 years ago
|
Attachment #8997146 -
Flags: review?(bugs)
Comment 14•6 years 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•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Updated•6 years ago
|
Assignee: nobody → dtownsend
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Updated•5 years ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•