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

Add semicolons to avoid unicode feature-detect generating typos #1823

Merged
merged 1 commit into from Jan 18, 2016

Conversation

@rowanthorpe
Copy link
Contributor

@rowanthorpe rowanthorpe commented Jan 5, 2016

The unicode feature-detect seems to introduce html typos (two html-entities without their trailing semicolons), which caused non-validating code for me. This PR fixes that.

@patrickkettner
Copy link
Member

@patrickkettner patrickkettner commented Jan 6, 2016

hey @rowanthorpe!
does non validating prevent it from functioning, or is it a conceptual purity thing?

@rowanthorpe
Copy link
Contributor Author

@rowanthorpe rowanthorpe commented Jan 6, 2016

BTW: I am serving my pages as polyglot XHTML5 (serving "application/xhtml+xml" to browsers that can use it) so this particular bug shows up in firebug as a syntax error in the js and as a subsequent well-formedness error in the xhtml. See attached partial-screenshot.

I think the answer to your question is "yes, and yes", because although the unicode-test would still work fine in svg-content (where backslash-notation is used), at a glance the typo would seem to always cause a negative result (often false) to the offsetWidth test in non-svg content (where the html-entities-without-semicolons are used). In xhtml mode the test-characters are apparently not rendered at all due to the well-formedness error, and I guess in non-xml-html they would be rendered as the actual strings "&#5987" and "&#9734" (either way they would be wrong) ...but I am not familiar with the code so take that as the quick impression that it is.

firebug-modernizr-unicode-bug_scrot

patrickkettner added a commit that referenced this pull request Jan 18, 2016
Add semicolons to avoid unicode feature-detect generating typos
@patrickkettner patrickkettner merged commit d9ed9de into Modernizr:master Jan 18, 2016
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@patrickkettner
Copy link
Member

@patrickkettner patrickkettner commented Jan 18, 2016

sorry it took so long to get this merged. I was trying to get xhtml5 testing built into the automated tests, but it isn't working in phantom. For whatever reason, the vml detect just won't work inside of a XHTML5 page, it ends up throwing a DOM Exception 11. It does work in chrome, and I ended up finding one other issue that I will be PRing momentarily.

@rowanthorpe rowanthorpe deleted the rowanthorpe:fix-missing-semicolons branch Jan 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants