fix(HtmlParser): mark <source> elements as void #5668
Conversation
|
@vicb picked up this one to get (a bit) familiar with the new HTML parser code. Not sure what you think about having tests for each specific tags |
pkozlowski-opensource
added
the
action: review
Not sure if it would be really useful - if you write the source and the tests, they would probably pass. however it doesn't hurt either. There are also more void elements that are not currently covered, could you add them ? (http://www.w3.org/TR/html5/syntax.html#void-elements) |
vicb
added
the
action: cleanup
| @@ -369,6 +369,7 @@ var TAG_DEFINITIONS: {[key: string]: HtmlTagDefinition} = { | |||
| 'rp': new HtmlTagDefinition({closedByChildren: ['rb', 'rt', 'rtc', 'rp'], closedByParent: true}), | |||
| 'optgroup': new HtmlTagDefinition({closedByChildren: ['optgroup'], closedByParent: true}), | |||
| 'option': new HtmlTagDefinition({closedByChildren: ['option', 'optgroup'], closedByParent: true}), | |||
| 'source': new HtmlTagDefinition({requiredParents: ['audio', 'video'], isVoid: true}), | |||
There was a problem hiding this comment.
requiredParents means that the first parent (audio) will be automatically added when none of them is present. I don't think it's like that in the spec, is it ?
pkozlowski-opensource
force-pushed
the
source_void
branch
from
bae0b59 to
092e8a0
Compare
pkozlowski-opensource
removed
the
action: cleanup
|
@vicb updated. Mind having another look? |
| // <command> - obsolete | ||
| // <keygen> - obsolete | ||
|
|
||
| humanizeDom(parser.parse('<map><area></map>', 'TestComp')); |
There was a problem hiding this comment.
[
'fix1',
'fix2',
].forEach((html) => {
expect(parser.parse(html, 'TestComp').errors).toEqual([]);
});
092e8a0 to
5ec317a
Compare
| @@ -75,6 +75,22 @@ export function main() { | |||
| ]); | |||
| }); | |||
|
|
|||
| it('should not error on void elements from HTML5 spec', | |||
| () => { | |||
|
|
|||
There was a problem hiding this comment.
nits:
- remove the extra blank lines
- have one fixture per line (I think that if you add a "," after the last element clang will not mess with the formatting)
- close the
.forEach()with a;
pkozlowski-opensource
force-pushed
the
source_void
branch
from
5ec317a to
e19dbae
Compare
|
@vicb clang-format seems to have very strong opinion about how things should work.... I'm not sure I find it easier to read now. But hey, this is why we've got formatter to make us all agree :-) Feel free to fiddle more with the formatting if you've got an idea on making it look nicer |
| '<div><embed></div>', '<div><hr></div>', '<div><img></div>', | ||
| '<div><input></div>', '<object><param>/<object>', '<audio><source></audio>', | ||
| '<audio><track></audio>', '<p><wbr></p>', | ||
| ].forEach((html) => { expect(parser.parse(html, 'TestComp').errors).toEqual([]); })}); |
pkozlowski-opensource
force-pushed
the
source_void
branch
from
e19dbae to
ce95e78
Compare
|
LGTM |
vicb
added
action: merge
|
Merging PR #5668 on behalf of @jelbourn to branch presubmit-jelbourn-pr-5668. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #5663