-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
fix: [#1615] Fixes problems related to parsing <html>, <head> and <body> using DOMParser #1617
fix: [#1615] Fixes problems related to parsing <html>, <head> and <body> using DOMParser #1617
Conversation
…dy> using DOMParser
…body> using DOMParser
const newDocument = domParser.parseFromString(DOMParserSVG, 'image/svg+xml'); | ||
expect(new XMLSerializer().serializeToString(newDocument).replace(/[\s]/gm, '')).toBe( | ||
DOMParserSVG.replace(/[\s]/gm, '') | ||
); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're planning on handling all the cases - then i suggest these as well:
export const DOMParserBODYSibling = '<body><example></example>Example Text</body><body></body>';
export const DOMParserBODYChildren = '<body><body></body><example></example>Example Text</body>';
it('recognises does not duplicate body when BODY is a child', () => {
const newDocument = domParser.parseFromString(DOMParserBODYChildren, 'text/html');
expect(newDocument.body.innerHTML).toBe('<example></example>Example Text');
});
it('recognises does not duplicate body when BODY is a sibling', () => {
const newDocument = domParser.parseFromString(DOMParserBODYSibling, 'text/html');
expect(newDocument.body.innerHTML).toBe('<example></example>Example Text');
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added the test cases, thank you! 🙂
This turned out to be a lot more complicated than what I first thought and I had to refactor quite a bit of the logic, but I have hopefully covered all cases reported and discovered when reading the spec and tested in the browser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's awesome though that you managed to get this through - impressive work :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @OlaviSau!
@@ -128,11 +138,13 @@ export default < | |||
}, | |||
col: { | |||
className: 'HTMLTableColElement', | |||
contentModel: HTMLElementConfigContentModelEnum.noDescendants | |||
contentModel: HTMLElementConfigContentModelEnum.noDescendants, | |||
permittedParents: ['colgroup'] | |||
}, | |||
colgroup: { | |||
className: 'HTMLTableColElement', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name is the same as the previous?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct.
If you run the following code in the browser you will see that both are using the HTMLTableColElement
class:
// Outputs "HTMLTableColElement"
console.log(document.createElement('col').constructor.name);
// Outputs "HTMLTableColElement"
console.log(document.createElement('colgroup').constructor.name);
…-thus-creates-it-twice
No description provided.