Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Issue while creating dash-delimited nodes #1987

Closed
LeonardoBraga opened this issue Jan 3, 2015 · 8 comments
Closed

Core: Issue while creating dash-delimited nodes #1987

LeonardoBraga opened this issue Jan 3, 2015 · 8 comments

Comments

@LeonardoBraga
Copy link
Contributor

JQuery fails in some cases to create dash-delimited notes:

// Works fine and creates the element:
$('<div-d></div-d>');

// Does not create the element:
$('<tr-d></tr-d>');

// ... and it should, since document.createElement works fine too:
document.createElement('tr-d');

Reported first here: angular/angular.js#10617

@markelog
Copy link
Member

markelog commented Jan 3, 2015

So we would be on same page here:
tr-d is not a valid html tag name, but since it has a hyphen in it, it is now considered as custom element and custom elements are part of web components.

Characters that could be used in tag name of custom elements are described in xml specification. Like _ and . are also included in that class. So if we fully support custom elements do we need to add tests for names with _ and . characters?

But this discussion could be deferred and i let @dmethvin answer that.

Apart from that, this looks like real use-case and real bug, there is two problems with this particular case: first with rsingleTag regexp and second one with rtagName one.

Your commit, mentioned in this ticket, would resolve <tr-d></tr-d> case, but not <tr-d></tr-d><tr-d></tr-d>.

This is why even div-d fails rsingleTag check, it still works, since buildFragment doesn't feel the need to wrap it with special parent tag.

Would you like to send us PR for both?

@markelog markelog added the Core label Jan 3, 2015
@markelog
Copy link
Member

markelog commented Jan 3, 2015

Would you like to send us PR for both?

Preferable with two commits in it, one for core and one for manipulation module, don't forget to read http://contribute.jquery.org/ if you feeling up to it

@LeonardoBraga
Copy link
Contributor Author

Thanks for the instructions, and yes, I'd like to submit a PR for it.

Just to be clear and fair, issue was initially reported and patched by @gkalpak on Angular.js repo. It just happened that I bumped into it at the same time and tried to share the load.

@gkalpak
Copy link

gkalpak commented Jan 3, 2015

To be even more clear and fair, @elaijuh first reported it on the AngularJS repo.
I submitted a PR for AngularJS (as did @richardaday).

@LeonardoBraga: I am glad you are sharing the load (since I am not familiar with jQuery's contributing conventions 😃).

(Seems like a whole lot of people are involved into this one-character addition 😆 How cool is that ?)

@elaijuh
Copy link

elaijuh commented Jan 4, 2015

cool to see this, thanks all for helping on this.

@gkalpak
Copy link

gkalpak commented Jan 16, 2015

Do we know in which version this is going to be included ?
(Sorry, I am not familiar with jQuery's release process.)

@markelog
Copy link
Member

Do we know in which version this is going to be included ?

3.0

@mgol
Copy link
Member

mgol commented Mar 7, 2016

This has been backported to 1.12.0 & 2.2.0 so I changed the milestone.

@mgol mgol added this to the 1.12.0/2.2.0 milestone Mar 7, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

5 participants