jqLite dumps <tr-xxx> while running jqLiteBuildFragment

[elaijuh]

$compile service will call jqLite to convert html to JQuery element. It's found that in jqLiteBuildFragment function, if a customized directive tag like <tr-xxx> will be wrapped to <table>... due to this regexp var TAG_NAME_REGEXP = /<([\w:]+)/; does not capture 'dash-delimited' tag name. And the fragment tr is in wrapMap which will be wrapped to a comprehensive table html later.

As a result, the directive cannot be compiled successfully.

I wonder any reason behind this or it's a bug to be fixed, should I PR for this?

---

https://github.com/angular/angular.js/blob/master/src/jqLite.js#L145

[gkalpak]

It looks like a pretty valid bug to me (easily fixed by changing the RegExp to /<([\w:-]+)/).
Is the - deliberately ignored or was it indeed just an oversight ?

[lgalfaso]

@gkalpak it was an oversight. If someone creates a PR, it should get merge fast

[gkalpak]

#10618 fixes it for jqLite, but jQuery still fails.
How should we address this ?

[gkalpak]

Upon further investigation, it seems like the RegExp and the implementation were based on jQuery (see here and there).
Not sure if making the same RegExp change makes sense for jQuery (or breaks other things there).
Should we look for alternative ways of solving this ? E.g. normalizing node names before passing to jqLiteBuildFragment/jqLiteParseHtml ?

[lgalfaso]

Mmm... it looks like this needs further investigation. @gkalpak do you want to take care of this? BTW, the issue with jQuery implies that #10618 as is, is not a good solution

[gkalpak]

#10618 will fix the issue when jqLite is used, but will not make any difference when jQuery is used instead (it keeps the current "buggy" behaviour). So, it won't hurt landing #10618 (althought there will be one test failing on jQuery), but it won't thoroughly solve the problem either.

Right now, jQuery does not properly handle constructing nodes from HTML text that has a root element with dash-delimited node name whose first subpart is a wrappable element.
The previous weird sentence in code:

$('<xyz></xyz>')          -> [<xyz></xyz>]          // OK
$('<xy-z></xy-z>')        -> [<xy-z></xy-z>]        // OK (`xy` is not a wrappable element.)
$('<tr-z></tr-z>')        -> []                     // Not OK (`tr` is a wrappable element.)
$('<x><tr-z></tr-z></x>') -> [<x><tr-z></tr-z></x>] // OK (`tr-z` is not the root element.)
$('<tr:z></tr:z>')        -> [<tr:z></tr:z>]        // OK (The node name is `:`-delimited.)

The way I see it, this is a bug with jQuery and should be fixed. I am not a jQuery guru, but from what I can tell the fix is the same as with jqLite (just adding - in the RegExp) and it shouldn't have any side-effects, since the RegExp is not used for any other purpose than to find if an element's node name is in the wrappable elements list).

So, if jQuery fixed this bug, then #10618 would be sufficient for solving the issue for Angular (with or without jQuery).

Not sure what is the indicated course of action. A few I can think of:

1. Sugmit a bug on the jQuery repo and merge fix(jqLite): properly handle dash-delimited node names in jqLiteBuildFragment #10618 once jQuery has fixed it. (Possibly submit a PR for jQuery to help speed things up.)
2. Keep the current behaviour (in order to retain jqLite-to-jQuery compatibility) and document the issue (i.e. that compiling (or even angular.element-ing) HTML strings that have a dash-delimited root element whose first subpart is a wrappable element does not work as expected). (Several work-arounds are available.)
3. Fix jqLite and document the buggy behaviour of jQuery. (Optionally combined with option (1).)
4. Find a way to handle this inside of Angular (e.g. normalizing dash-delimited node names before passing to angular.element/compile ?
   Since all this boils down to jqLite('...'), I am not ure how easy it would be to handle this in the presence of jQuery.

[lgalfaso]

I would start with opening an issue with jQuery (to include a PR would be ideal) and see how things move from that side... and until there is a definition from jQeury, I would leave #10618 on hold

[gkalpak]

Will do over the weekend.

[gkalpak]

@LeonardoBraga beat me to it 😄

[petebacondarwin]

We have a fix ready to be merged at #12759