Naming Custom Elements in browser chrome

[Brian Grinstead]

One of the things to figure out before we can migrate the first XBL binding to a Custom Element is what to do about naming the element. By spec Custom Elements need to have a dash in their name, but our existing XUL elements typically don't.

That means we could either:

1) Change the platform code to allow non-dashed Custom Element names for chrome content.
2) Rewrite the frontend code to use a dashed version of element names while migrating the binding. For example, we could rename '<deck>' to '<moz-deck>'.

A couple of drawbacks for (1) are:
* We'd have to find a fast way to do it. If we could trust 'IsParentProcess' to mean 'browser chrome' that would work, but that's complicated by (a) Fennec doesn't use e10s and (b) we still have prefs and environment variables that allow e10s to be turned off.
* It diverges from the spec, so it's another special thing to know about when working on browser chrome compared to content.

A couple of drawbacks for (2) are:
* It would slow down progress, since we'd need to be rebasing around the entire frontend + tests while migrating a binding. And all callers aren't necessarily easy to find - you'd have to search for `<deck`, `<xul:deck`, `document.createElement("deck")`, `document.createElementNS("deck")` and so on.
* We'd need to update documentation to reflect the new name.

To make migrating bindings easier I'd prefer (1) if we can make it fast, but in the end I'd like to get to (2) so we are closer to the normal behavior. Are there options I haven't thought of? Or opinions one way or another?

Thanks,
Brian
_______________________________________________
firefox-dev mailing list
firef...@mozilla.org
https://mail.mozilla.org/listinfo/firefox-dev

[Boris Zbarsky]

On 3/12/18 5:40 PM, Brian Grinstead wrote:
> * It would slow down progress, since we'd need to be rebasing around the entire frontend + tests while migrating a binding.

We could add a hack to NS_NewXULElement that maps some tag names to
others (and creates a new nodeinfo accordingly). This would be used
only on a temporary basis, while migrating a binding. So the workflow
would go like this:

1) Add a mapping here.
2) Do the binding to custom element migration and land it.
3) Go through and convert all the tag names to the dashed version,
landing the conversions as you go.
4) Add a fatal assert (diagnostic or release as desired) for when the
mapping gets hit. Convert anything that this assert turns up.
4) Remove the mapping.

That won't help with things created via custom element ctors, but that's
OK by definition. It also won't help with things created via
NS_TrustedNewXULElement, but we only have 7 things that are (scrollbar
bits, parts of file and data/time controls, and videocontrols). When we
need to deal with those, we can.

This will slow down XUL element creation a bit, but not much, I expect;
linear search over a small array in NS_NewXULElement should be fine.

Anyway, this is sort of a combination of (1) and (2) that might get you
the best of both worlds.

> And all callers aren't necessarily easy to find - you'd have to search for `<deck`, `<xul:deck`, `document.createElement("deck")`, `document.createElementNS("deck")` and so on.

If we're trying to find them by code inspection, that's true. If we
think they all live on codepaths reached in testing, the assert approach
above is a way to find them.

-Boris

[Anne van Kesteren]

On Mon, Mar 12, 2018 at 10:40 PM, Brian Grinstead
<bgrin...@mozilla.com> wrote:
> * It diverges from the spec, so it's another special thing to know about when working on browser chrome compared to content.

We could invest in figuring out
https://github.com/w3c/webcomponents/issues/634. I think we could even
make it so that if your element is not in the HTML, SVG, or MathML
namespace, you don't need a hyphen for its local name. Now if
something like that works well together with our XUL built-ins I'm not
sure. And it's also somewhat unclear whether such an investment is of
interest to other browsers given that the web largely does not care
for non-HTML/SVG/MathML, so it might not end up being standardized
anyway.


--
https://annevankesteren.nl/

[Dave Townsend]

On Mon, Mar 12, 2018 at 8:20 PM Boris Zbarsky <bzba...@mit.edu> wrote:

On 3/12/18 5:40 PM, Brian Grinstead wrote:
> * It would slow down progress, since we'd need to be rebasing around the entire frontend + tests while migrating a binding.

We could add a hack to NS_NewXULElement that maps some tag names to
others (and creates a new nodeinfo accordingly).  This would be used
only on a temporary basis, while migrating a binding.  So the workflow
would go like this:

This is roughly an idea I also had. I was wondering though if that would mean that the element would show up in the inspector with the mapped tag name and whether we'd hit issues with CSS not applying correctly.

[Dave Townsend]

On Mon, Mar 12, 2018 at 8:55 PM Anne van Kesteren <ann...@annevk.nl> wrote:

On Mon, Mar 12, 2018 at 10:40 PM, Brian Grinstead
<bgrin...@mozilla.com> wrote:
> * It diverges from the spec, so it's another special thing to know about when working on browser chrome compared to content.

We could invest in figuring out
https://github.com/w3c/webcomponents/issues/634. I think we could even
make it so that if your element is not in the HTML, SVG, or MathML
namespace, you don't need a hyphen for its local name. Now if
something like that works well together with our XUL built-ins I'm not
sure. And it's also somewhat unclear whether such an investment is of
interest to other browsers given that the web largely does not care
for non-HTML/SVG/MathML, so it might not end up being standardized
anyway.

Ultimately the goal is to ditch XUL entirely so eventually we'd want these elements to be in the HTML namespace. So this approach might be useful as a stopgap but probably not longterm.

[Boris Zbarsky]

On 3/13/18 4:04 PM, Dave Townsend wrote:
> I was wondering though if that would
> mean that the element would show up in the inspector with the mapped tag
> name

Yes. That seems to be ok if that's the desired end state anyway.

> and whether we'd hit issues with CSS not applying correctly.

Right, would need to make sure to update CSS first to style both
tagnames. Good catch.

[Brian Grinstead]

If we could make relaxing the naming rules based on namespace fast, that does seem like a simpler temporary state than modifying the tag name when creating the element. Then we could still use the rest of the proposed workflow. So, first change nsContentUtils::IsCustomElementName to allow these names in XUL namespace (either using a whitelist of names or by allowing all non-dashed names). Then for each binding:

1) Do the binding to custom element migration and land it.

2) Go through and convert all the tag names to the dashed version, landing the conversions as you go.

3) Add a fatal assert (diagnostic or release as desired) in NS_NewXULElement for when we attempt to create the non-dashed version. Convert anything that this assert turns up.

4) (If we used a whitelist of names in IsCustomElementName) remove the name from the whitelist.

[Boris Zbarsky]

On 3/12/18 11:55 PM, Anne van Kesteren wrote:
> Now ifsomething like that works well together with our XUL built-ins I'm not
> sure.

We only have one type of XUL element, fwiw. So there's no real concept
of "XUL built-ins". This makes the story a lot simpler than HTML's.

-Boris

[Boris Zbarsky]

On 3/13/18 4:26 PM, Brian Grinstead wrote:
> If we could make relaxing the naming rules based on namespace fast

Can we define "fast"?

Checking the namespace is fast.

The work that happens for every element creation for elements that might
be custom elements may not be so fast.

> (either using a whitelist of names or by allowing all non-dashed names).

The latter is faster to check but slows down creations more. The former
is equivalent to my proposal perf-wise.

> 1) Do the binding to custom element migration and land it.
> 2) Go through and convert all the tag names to the dashed version,
> landing the conversions as you go.

This part I'm not following. Which tag name is the custom element
definition attached to? It can't be attached to both, iirc....

-Boris

[Brian Grinstead]

> On Mar 14, 2018, at 7:44 AM, Boris Zbarsky <bzba...@mit.edu> wrote:
>
> On 3/13/18 4:26 PM, Brian Grinstead wrote:
>> If we could make relaxing the naming rules based on namespace fast
>
> Can we define "fast"?
>
> Checking the namespace is fast.
>
> The work that happens for every element creation for elements that might be custom elements may not be so fast.

>> (either using a whitelist of names or by allowing all non-dashed names).
>
> The latter is faster to check but slows down creations more. The former is equivalent to my proposal perf-wise.

I don’t know how much extra work we have to do for elements that may be custom elements, but we can do some talos pushes to compare the two approaches. If we added elements to the whitelist right as we convert them and then remove them after the tags are renamed, I doubt that whitelist should ever get too long. The downside is then we can’t do the initial frontend migration in artifact builds, but that’s not a deal-breaker. Or another option that may split the difference would be to have a blacklist of the most common XUL elements that aren’t currently or aren’t going to be custom elements (i.e. hbox, vbox, label, image) and consider everything else valid.

>> 1) Do the binding to custom element migration and land it.
>> 2) Go through and convert all the tag names to the dashed version, landing the conversions as you go.
>

> This part I'm not following. Which tag name is the custom element definition attached to? It can't be attached to both, iirc….

Yeah, thinking about it more in this version steps (2) and (3) would have to happen all at once in one patch, i.e. no “landing the conversions as you go”. That means we can’t split up the tag renaming into multiple bugs, but as long as it isn’t coupled with the actual binding->CE migration bug I think that’s OK. And I guess (3) would be optional, but having that assertion at least for a temporary period still seems useful in case some system addon or another usage that wasn’t covered by the test suite comes up in practice.

Brian

[Brian Grinstead]

Thanks all for the input - it sounds like we have a couple of good options for handling this so I filed Bug 1446247 to sort out the details.

Brian