Check for node type before calling normalize.#491
Check for node type before calling normalize.#491andrewplummer wants to merge 4 commits intojsdom:masterfrom andrewplummer:master
Conversation
|
Hi, I don't quite understand what the problem is here. Does this only occur in the presence of libraries that mess with |
Will try to add a failing test. Thanks for the response. |
|
Sounds good! Probably the right place to add the test is at the bottom of test/level2/html.js. (I'm still figuring my own way around the repo, to be honest.) |
"normalize" on string instances.
|
Okay, that should do it. I've confirmed that that test will fail without the fix. Both the test and the fix are pretty minimal. The test should be fine, but I don't expect the fix to necessarily be ideal (but it does get the job done here). Thank you for addressing this! |
|
Merged, thanks! |
|
Sorry, I had to revert this because it was causing test failures in level2/html. Please feel free to resubmit if you can make all the tests pass. |
|
ok I think this latest test should fix it... I'm now just doing a |
|
OK cool, merged! |
Sugar defines a method
normalizeon strings. Lines 165-199 ofhtml.jsappear to step through the attributes of a node, and call a anormalizemethod that is intended to be defined elsewhere in an "attribute" object. That in itself should not be an issue, but it appears that certain attributes may in fact be strings. I am currently encountering this in thesrcattribute ofimgnodes, however interestingly not all node attributes exhibit this behavior, for exampletypeattributes oninputelements are fine. At any rate it seems that either strings are not intended here orn.normalizeshould be performing a type check before calling that method. The change I made is rather naive, and I wouldn't be surprised if there's a better check that can be performed, but it gets the job done for me.Note that this is currently being tested using Apricot and node. This will fix andrewplummer/Sugar#188