Skip to content

Commit

Permalink
feat(HtmlParser): enforce no end tag for void elements
Browse files Browse the repository at this point in the history 
BREAKING CHANGE

End tags used to be tolerated for void elements with no content.
They are no more allowed so that we more closely follow the HTML5 spec.
  • Loading branch information
@vicb
vicb committed Dec 4, 2015
1 parent b925ff5 commit 5660446
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 53 deletions.
17 changes: 7 additions & 10 deletions modules/angular2/src/compiler/html_parser.ts
View file
Expand Up @@ -171,17 +171,14 @@ class TreeBuilder {
private _consumeEndTag(endTagToken: HtmlToken) {
var fullName =
getElementFullName(endTagToken.parts[0], endTagToken.parts[1], this._getParentElement());
if (!this._popElement(fullName)) {
let msg;

if (getHtmlTagDefinition(fullName).isVoid) {
msg =
`Void elements do not have end tags (they can not have content) "${endTagToken.parts[1]}"`;
} else {
msg = `Unexpected closing tag "${endTagToken.parts[1]}"`;
}

this.errors.push(HtmlTreeError.create(fullName, endTagToken.sourceSpan.start, msg));
if (getHtmlTagDefinition(fullName).isVoid) {
this.errors.push(
HtmlTreeError.create(fullName, endTagToken.sourceSpan.start,
`Void elements do not have end tags "${endTagToken.parts[1]}"`));
} else if (!this._popElement(fullName)) {
this.errors.push(HtmlTreeError.create(fullName, endTagToken.sourceSpan.start,
`Unexpected closing tag "${endTagToken.parts[1]}"`));
}
}

Expand Down
30 changes: 3 additions & 27 deletions modules/angular2/test/compiler/html_parser_spec.ts
View file
Expand Up @@ -85,11 +85,6 @@ export function main() {
]);
});

it('should tolerate end tags for void elements when they have no content', () => {
expect(humanizeDom(parser.parse('<input></input>', 'TestComp')))
.toEqual([[HtmlElementAst, 'input', 0]]);
});

it('should support optional end tags', () => {
expect(humanizeDom(parser.parse('<div><p>1<p>2</div>', 'TestComp')))
.toEqual([
Expand Down Expand Up @@ -214,30 +209,11 @@ export function main() {
expect(humanizeErrors(errors)).toEqual([['p', 'Unexpected closing tag "p"', '0:5']]);
});

it('should report text content in void elements', () => {
let errors = parser.parse('<input>content</input>', 'TestComp').errors;
expect(errors.length).toEqual(1);
expect(humanizeErrors(errors))
.toEqual([
[
'input',
'Void elements do not have end tags (they can not have content) "input"',
'0:14'
]
]);
});

it('should report html content in void elements', () => {
let errors = parser.parse('<input><p></p></input>', 'TestComp').errors;
it('should report closing tag for void elements', () => {
let errors = parser.parse('<input></input>', 'TestComp').errors;
expect(errors.length).toEqual(1);
expect(humanizeErrors(errors))
.toEqual([
[
'input',
'Void elements do not have end tags (they can not have content) "input"',
'0:14'
]
]);
.toEqual([['input', 'Void elements do not have end tags "input"', '0:7']]);
});

it('should also report lexer errors', () => {
Expand Down
5 changes: 2 additions & 3 deletions modules/angular2/test/compiler/template_normalizer_spec.ts
View file
Expand Up @@ -241,7 +241,7 @@ export function main() {
var template = normalizer.normalizeLoadedTemplate(
dirType,
new CompileTemplateMetadata({encapsulation: null, styles: [], styleUrls: []}),
'<link href="b" rel="a"></link>', 'package:some/module/');
'<link href="b" rel="a">', 'package:some/module/');
expect(template.styleUrls).toEqual([]);
}));

Expand All @@ -250,8 +250,7 @@ export function main() {
var template = normalizer.normalizeLoadedTemplate(
dirType,
new CompileTemplateMetadata({encapsulation: null, styles: [], styleUrls: []}),
'<link href="http://some/external.css" rel="stylesheet"></link>',
'package:some/module/');
'<link href="http://some/external.css" rel="stylesheet">', 'package:some/module/');
expect(template.styleUrls).toEqual([]);
}));

Expand Down
17 changes: 7 additions & 10 deletions modules/angular2/test/compiler/template_parser_spec.ts
View file
Expand Up @@ -774,8 +774,7 @@ Property binding a not used by any directive on an embedded template ("[ERROR ->

it('should keep <link rel="stylesheet"> elements if they have an absolute non package: url',
() => {
expect(
humanizeTplAst(parse('<link rel="stylesheet" href="http://someurl"></link>a', [])))
expect(humanizeTplAst(parse('<link rel="stylesheet" href="http://someurl">a', [])))
.toEqual([
[ElementAst, 'link'],
[AttrAst, 'rel', 'stylesheet'],
Expand All @@ -785,22 +784,21 @@ Property binding a not used by any directive on an embedded template ("[ERROR ->
});

it('should keep <link rel="stylesheet"> elements if they have no uri', () => {
expect(humanizeTplAst(parse('<link rel="stylesheet"></link>a', [])))
expect(humanizeTplAst(parse('<link rel="stylesheet">a', [])))
.toEqual([[ElementAst, 'link'], [AttrAst, 'rel', 'stylesheet'], [TextAst, 'a']]);
expect(humanizeTplAst(parse('<link REL="stylesheet"></link>a', [])))
expect(humanizeTplAst(parse('<link REL="stylesheet">a', [])))
.toEqual([[ElementAst, 'link'], [AttrAst, 'REL', 'stylesheet'], [TextAst, 'a']]);
});

it('should ignore <link rel="stylesheet"> elements if they have a relative uri', () => {
expect(humanizeTplAst(parse('<link rel="stylesheet" href="./other.css"></link>a', [])))
expect(humanizeTplAst(parse('<link rel="stylesheet" href="./other.css">a', [])))
.toEqual([[TextAst, 'a']]);
expect(humanizeTplAst(parse('<link rel="stylesheet" HREF="./other.css"></link>a', [])))
expect(humanizeTplAst(parse('<link rel="stylesheet" HREF="./other.css">a', [])))
.toEqual([[TextAst, 'a']]);
});

it('should ignore <link rel="stylesheet"> elements if they have a package: uri', () => {
expect(humanizeTplAst(
parse('<link rel="stylesheet" href="package:somePackage"></link>a', [])))
expect(humanizeTplAst(parse('<link rel="stylesheet" href="package:somePackage">a', [])))
.toEqual([[TextAst, 'a']]);
});

Expand Down Expand Up @@ -835,8 +833,7 @@ Property binding a not used by any directive on an embedded template ("[ERROR ->

it('should ignore <link rel="stylesheet"> elements inside of elements with ng-non-bindable',
() => {
expect(humanizeTplAst(
parse('<div ng-non-bindable><link rel="stylesheet"></link>a</div>', [])))
expect(humanizeTplAst(parse('<div ng-non-bindable><link rel="stylesheet">a</div>', [])))
.toEqual([[ElementAst, 'div'], [AttrAst, 'ng-non-bindable', ''], [TextAst, 'a']]);
});

Expand Down
2 changes: 1 addition & 1 deletion modules/angular2/test/core/linker/integration_spec.ts
View file
Expand Up @@ -1007,7 +1007,7 @@ export function main() {
tcb.overrideView(
MyComp, new ViewMetadata({
template:
'<input type="checkbox" listenerprevent></input><input type="checkbox" listenernoprevent></input>',
'<input type="checkbox" listenerprevent><input type="checkbox" listenernoprevent>',
directives: [
DirectiveListeningDomEventPrevent,
DirectiveListeningDomEventNoPrevent
Expand Down
3 changes: 1 addition & 2 deletions modules/angular2/test/web_workers/worker/renderer_integration_spec.ts
View file
Expand Up @@ -211,8 +211,7 @@ export function main() {
it('should call actions on the element independent of the compilation',
inject([TestComponentBuilder, Renderer, AsyncTestCompleter],
(tcb: TestComponentBuilder, renderer: Renderer, async) => {
tcb.overrideView(MyComp,
new ViewMetadata({template: '<input [title]="y"></input>'}))
tcb.overrideView(MyComp, new ViewMetadata({template: '<input [title]="y">'}))
.createAsync(MyComp)
.then((fixture) => {
var elRef = fixture.debugElement.componentViewChildren[0].elementRef;
Expand Down

0 comments on commit 5660446

Please sign in to comment.