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

Stop passing null as second argument to document.createElement() #6896

Merged
merged 2 commits into from
May 26, 2016

Conversation

darobin
Copy link
Contributor

@darobin darobin commented May 26, 2016

When using the two-parameter variant of document.createElement() (which is used for Custom Elements), if null is passed as the second parameter then Firefox will stringify it to the DOM as an is="null" attribute. I don't believe that this causes any manner of problem, but it is surprising (as exemplified in this discussion thread.

This change simply changes the call to avoid passing null.

@gaearon
Copy link
Collaborator

gaearon commented May 26, 2016

Can we just always pass props.is? It would be undefined unless specified, which is indistinguishable from not passing it at all.

cc @jimfb

@darobin
Copy link
Contributor Author

darobin commented May 26, 2016

I tried that, it was my first guess: Firefox helpfully gives you is="undefined" in return.

@gaearon
Copy link
Collaborator

gaearon commented May 26, 2016

Haha, got it. Let’s split into two branches to make it a bit easier on eyes?

} else if (props.is) {
  ...
} else {
  ...
}

@darobin
Copy link
Contributor Author

darobin commented May 26, 2016

Sure thing, done.

@gaearon
Copy link
Collaborator

gaearon commented May 26, 2016

Is there any way to test this with jsdom so we don’t regress?

@ghost
Copy link

ghost commented May 26, 2016

@darobin updated the pull request.

@darobin
Copy link
Contributor Author

darobin commented May 26, 2016

Not that I can think of, at least not without intercepting createElement, which is doable but perhaps a bit brutal. Note that Firefox's behaviour is incorrect as per (current) spec, and therefore likely to change at some point.

@gaearon gaearon merged commit 2636155 into facebook:master May 26, 2016
@gaearon
Copy link
Collaborator

gaearon commented May 26, 2016

Thanks!

@zpao
Copy link
Member

zpao commented May 27, 2016

Do you know if this Firefox bug been reported to Mozilla?

@zpao zpao added this to the 15.y.z milestone May 27, 2016
@darobin
Copy link
Contributor Author

darobin commented May 27, 2016

Yes, I discussed it on #whatwg this morning to double-check, and reported it in bugzilla.

zpao pushed a commit to zpao/react that referenced this pull request Jun 8, 2016
…ebook#6896)

* Stop passing null as second argument to document.createElement()

* rewrap check for props.is to make it more readable

(cherry picked from commit 2636155)
zpao pushed a commit that referenced this pull request Jun 14, 2016
* Stop passing null as second argument to document.createElement()

* rewrap check for props.is to make it more readable

(cherry picked from commit 2636155)
@zpao zpao modified the milestones: 15-next, 15.2.0 Jun 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants