Brian Grinstead [:bgrins] Reporter
	Description • 6 years 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().

	Dave Townsend [:mossop] 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?

	Olli Pettay [:smaug][bugs@pettay.fi]
	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)

	Brian Grinstead [:bgrins] 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.

	Dave Townsend [:mossop] 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.

	Dave Townsend [:mossop] 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?

	Olli Pettay [:smaug][bugs@pettay.fi]
	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.

	Brian Grinstead [:bgrins] 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).

	Dave Townsend [:mossop] 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?

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 9 • 6 years ago

What about creating new elements use new Foo() ? That is what I'm more worried, element creation time.

	Dave Townsend [:mossop] 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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 11 • 6 years ago

oops, I should be looking at the patch when commenting again.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 12 • 6 years ago

But ok, then looks reasonable.

	Olli Pettay [:smaug][bugs@pettay.fi]
	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.

	Pulsebot
	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

	Bogdan Tara[:bogdan_tara | bogdant]
	Comment 15 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/413c0f58374c