From 56604468e099f7031f4f8eaefff76e3e467dae26 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 3 Dec 2015 15:53:44 -0800 Subject: [PATCH] feat(HtmlParser): enforce no end tag for void elements 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. --- modules/angular2/src/compiler/html_parser.ts | 17 +++++------ .../test/compiler/html_parser_spec.ts | 30 ++----------------- .../test/compiler/template_normalizer_spec.ts | 5 ++-- .../test/compiler/template_parser_spec.ts | 17 +++++------ .../test/core/linker/integration_spec.ts | 2 +- .../worker/renderer_integration_spec.ts | 3 +- 6 files changed, 21 insertions(+), 53 deletions(-) diff --git a/modules/angular2/src/compiler/html_parser.ts b/modules/angular2/src/compiler/html_parser.ts index b9333c0295..cf3474ba02 100644 --- a/modules/angular2/src/compiler/html_parser.ts +++ b/modules/angular2/src/compiler/html_parser.ts @@ -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]}"`)); } } diff --git a/modules/angular2/test/compiler/html_parser_spec.ts b/modules/angular2/test/compiler/html_parser_spec.ts index f851c1ec1c..d779f0dd4b 100644 --- a/modules/angular2/test/compiler/html_parser_spec.ts +++ b/modules/angular2/test/compiler/html_parser_spec.ts @@ -85,11 +85,6 @@ export function main() { ]); }); - it('should tolerate end tags for void elements when they have no content', () => { - expect(humanizeDom(parser.parse('', 'TestComp'))) - .toEqual([[HtmlElementAst, 'input', 0]]); - }); - it('should support optional end tags', () => { expect(humanizeDom(parser.parse('

1

2

', 'TestComp'))) .toEqual([ @@ -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('content', 'TestComp').errors; + it('should report closing tag for void elements', () => { + let errors = parser.parse('', '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('

', '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', () => { diff --git a/modules/angular2/test/compiler/template_normalizer_spec.ts b/modules/angular2/test/compiler/template_normalizer_spec.ts index c90ab17046..5d1182a326 100644 --- a/modules/angular2/test/compiler/template_normalizer_spec.ts +++ b/modules/angular2/test/compiler/template_normalizer_spec.ts @@ -241,7 +241,7 @@ export function main() { var template = normalizer.normalizeLoadedTemplate( dirType, new CompileTemplateMetadata({encapsulation: null, styles: [], styleUrls: []}), - '', 'package:some/module/'); + '', 'package:some/module/'); expect(template.styleUrls).toEqual([]); })); @@ -250,8 +250,7 @@ export function main() { var template = normalizer.normalizeLoadedTemplate( dirType, new CompileTemplateMetadata({encapsulation: null, styles: [], styleUrls: []}), - '', - 'package:some/module/'); + '', 'package:some/module/'); expect(template.styleUrls).toEqual([]); })); diff --git a/modules/angular2/test/compiler/template_parser_spec.ts b/modules/angular2/test/compiler/template_parser_spec.ts index 574c054e10..b9901638e9 100644 --- a/modules/angular2/test/compiler/template_parser_spec.ts +++ b/modules/angular2/test/compiler/template_parser_spec.ts @@ -774,8 +774,7 @@ Property binding a not used by any directive on an embedded template ("[ERROR -> it('should keep elements if they have an absolute non package: url', () => { - expect( - humanizeTplAst(parse('a', []))) + expect(humanizeTplAst(parse('a', []))) .toEqual([ [ElementAst, 'link'], [AttrAst, 'rel', 'stylesheet'], @@ -785,22 +784,21 @@ Property binding a not used by any directive on an embedded template ("[ERROR -> }); it('should keep elements if they have no uri', () => { - expect(humanizeTplAst(parse('a', []))) + expect(humanizeTplAst(parse('a', []))) .toEqual([[ElementAst, 'link'], [AttrAst, 'rel', 'stylesheet'], [TextAst, 'a']]); - expect(humanizeTplAst(parse('a', []))) + expect(humanizeTplAst(parse('a', []))) .toEqual([[ElementAst, 'link'], [AttrAst, 'REL', 'stylesheet'], [TextAst, 'a']]); }); it('should ignore elements if they have a relative uri', () => { - expect(humanizeTplAst(parse('a', []))) + expect(humanizeTplAst(parse('a', []))) .toEqual([[TextAst, 'a']]); - expect(humanizeTplAst(parse('a', []))) + expect(humanizeTplAst(parse('a', []))) .toEqual([[TextAst, 'a']]); }); it('should ignore elements if they have a package: uri', () => { - expect(humanizeTplAst( - parse('a', []))) + expect(humanizeTplAst(parse('a', []))) .toEqual([[TextAst, 'a']]); }); @@ -835,8 +833,7 @@ Property binding a not used by any directive on an embedded template ("[ERROR -> it('should ignore elements inside of elements with ng-non-bindable', () => { - expect(humanizeTplAst( - parse('
a
', []))) + expect(humanizeTplAst(parse('
a
', []))) .toEqual([[ElementAst, 'div'], [AttrAst, 'ng-non-bindable', ''], [TextAst, 'a']]); }); diff --git a/modules/angular2/test/core/linker/integration_spec.ts b/modules/angular2/test/core/linker/integration_spec.ts index c6313b3d1f..f647a1cf4e 100644 --- a/modules/angular2/test/core/linker/integration_spec.ts +++ b/modules/angular2/test/core/linker/integration_spec.ts @@ -1007,7 +1007,7 @@ export function main() { tcb.overrideView( MyComp, new ViewMetadata({ template: - '', + '', directives: [ DirectiveListeningDomEventPrevent, DirectiveListeningDomEventNoPrevent diff --git a/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts b/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts index 65d3b9ab83..cf7e14de2f 100644 --- a/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts +++ b/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts @@ -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: ''})) + tcb.overrideView(MyComp, new ViewMetadata({template: ''})) .createAsync(MyComp) .then((fixture) => { var elRef = fixture.debugElement.componentViewChildren[0].elementRef;