fix: Restore behavior from v0.1.27#67
Conversation
Might be considered a breaking change. fixes xmldom#57
|
Thanks. This would definitely be breaking for anything that does not expect |
|
I think it's a question of definition. Your decision. |
brody2consult
left a comment
brody2consult
left a comment
There was a problem hiding this comment.
Approved to be merged after PR #77 - it would be nice for me to validate that the Stryker dashboard is updated with this change.
I do have one non-blocking suggestion.
| @@ -13,13 +13,8 @@ wows.describe('html normalizer').addBatch({ | |||
| assert(dom+'', '<div xmlns="http://www.w3.org/1999/xhtml"><123e>&<a<br/></div>'); | |||
|
|
|||
| var dom = new DOMParser().parseFromString('<div> © ©</div>','text/html'); | |||
There was a problem hiding this comment.
| var dom = new DOMParser().parseFromString('<div> © ©</div>','text/html'); | |
| var dom = new DOMParser().parseFromString('<div>© ©</div>>','text/html'); |
(simplify the test string and fix the indentation)
Or do we need to keep the " ©" part to test something? If so, I would kinda favor separate test strings, with separate test assertions, to check the behavior and to test with " ©".
I do think it would be better to break this test script into smaller test cases, and potentially multiple files, at some point.
TODO: The assertion would have to be updated as well. Unfortunately the GitHub suggested changes feature does not work for multiple lines if some lines were deleted in the middle.
There was a problem hiding this comment.
I agree that the tests are not ideal, that's why I raised #72 to discuss the options.
I would avoid to mix this into other PRs to not loose track.
| assert(dom+'', '<div xmlns="http://www.w3.org/1999/xhtml"> \u00a9&nbsp&copy</div>'); | ||
| assert(dom+'', '<div xmlns="http://www.w3.org/1999/xhtml">\u00a0\u00a9&nbsp&copy</div>'); | ||
|
|
||
|
|
|
I went ahead with the merge, and the change does now show up on the Stryker dashboard. Thanks! |
2 participants
Might be considered a breaking change.
fixes #57