From 4811dc6eed9809738aa18c4dc02809bcdc18de03 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 7 Dec 2015 09:41:01 -0800 Subject: [PATCH 1/4] fix(HtmlParser): Do not add parent element for template children fixes #5638 --- modules/angular2/src/compiler/html_tags.ts | 12 ++++++++++-- modules/angular2/test/compiler/html_parser_spec.ts | 8 ++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/modules/angular2/src/compiler/html_tags.ts b/modules/angular2/src/compiler/html_tags.ts index a2b77fef3d0a..e45ff34d163c 100644 --- a/modules/angular2/src/compiler/html_tags.ts +++ b/modules/angular2/src/compiler/html_tags.ts @@ -304,8 +304,16 @@ export class HtmlTagDefinition { } requireExtraParent(currentParent: string): boolean { - return isPresent(this.requiredParents) && - (isBlank(currentParent) || this.requiredParents[currentParent.toLowerCase()] != true); + if (isBlank(this.requiredParents)) { + return false; + } + + if (isBlank(currentParent)) { + return true; + } + + let lcParent = currentParent.toLowerCase(); + return this.requiredParents[lcParent] != true && lcParent != 'template'; } isClosedByChild(name: string): boolean { diff --git a/modules/angular2/test/compiler/html_parser_spec.ts b/modules/angular2/test/compiler/html_parser_spec.ts index eb709c6113aa..a9217414f8ee 100644 --- a/modules/angular2/test/compiler/html_parser_spec.ts +++ b/modules/angular2/test/compiler/html_parser_spec.ts @@ -141,6 +141,14 @@ export function main() { ]); }); + it('should not add the requiredParent when the parent is a template', () => { + expect(humanizeDom(parser.parse('', 'TestComp'))) + .toEqual([ + [HtmlElementAst, 'template', 0], + [HtmlElementAst, 'tr', 1], + ]); + }); + it('should support explicit mamespace', () => { expect(humanizeDom(parser.parse('', 'TestComp'))) .toEqual([[HtmlElementAst, '@myns:div', 0]]); From 12ec1b408ae8c4a825d5845e2cd21afeba22f7ad Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Sun, 6 Dec 2015 13:11:00 -0800 Subject: [PATCH 2/4] feat(HtmlLexer): allow "<" in text tokens fixes #5550 --- modules/angular2/src/compiler/html_lexer.ts | 68 +++++++++++++++---- .../angular2/test/compiler/html_lexer_spec.ts | 53 ++++++++------- 2 files changed, 85 insertions(+), 36 deletions(-) diff --git a/modules/angular2/src/compiler/html_lexer.ts b/modules/angular2/src/compiler/html_lexer.ts index 243a5e69b432..e3a725ce9d71 100644 --- a/modules/angular2/src/compiler/html_lexer.ts +++ b/modules/angular2/src/compiler/html_lexer.ts @@ -6,6 +6,7 @@ import { CONST_EXPR, serializeEnum } from 'angular2/src/facade/lang'; +import {ListWrapper} from 'angular2/src/facade/collection'; import {ParseLocation, ParseError, ParseSourceFile, ParseSourceSpan} from './parse_util'; import {getHtmlTagDefinition, HtmlTagContentType, NAMED_ENTITIES} from './html_tags'; @@ -161,7 +162,7 @@ class _HtmlTokenizer { } this._beginToken(HtmlTokenType.EOF); this._endToken([]); - return new HtmlTokenizeResult(this.tokens, this.errors); + return new HtmlTokenizeResult(mergeTextTokens(this.tokens), this.errors); } private _getLocation(): ParseLocation { @@ -374,21 +375,37 @@ class _HtmlTokenizer { } private _consumeTagOpen(start: ParseLocation) { - this._attemptUntilFn(isNotWhitespace); - var nameStart = this.index; - this._consumeTagOpenStart(start); - var lowercaseTagName = this.inputLowercase.substring(nameStart, this.index); - this._attemptUntilFn(isNotWhitespace); - while (this.peek !== $SLASH && this.peek !== $GT) { - this._consumeAttributeName(); + let savedPos = this._savePosition(); + let lowercaseTagName; + try { + this._attemptUntilFn(isNotWhitespace); + var nameStart = this.index; + this._consumeTagOpenStart(start); + lowercaseTagName = this.inputLowercase.substring(nameStart, this.index); this._attemptUntilFn(isNotWhitespace); - if (this._attemptChar($EQ)) { + while (this.peek !== $SLASH && this.peek !== $GT) { + this._consumeAttributeName(); + this._attemptUntilFn(isNotWhitespace); + if (this._attemptChar($EQ)) { + this._attemptUntilFn(isNotWhitespace); + this._consumeAttributeValue(); + } this._attemptUntilFn(isNotWhitespace); - this._consumeAttributeValue(); } - this._attemptUntilFn(isNotWhitespace); + this._consumeTagOpenEnd(); + } catch (e) { + if (e instanceof ControlFlowError) { + // When the start tag is invalid, assume we want a "<" + this._restorePosition(savedPos); + // Back to back text tokens are merged at the end + this._beginToken(HtmlTokenType.TEXT, start); + this._endToken(['<']); + return; + } + + throw e; } - this._consumeTagOpenEnd(); + var contentTokenType = getHtmlTagDefinition(lowercaseTagName).contentType; if (contentTokenType === HtmlTagContentType.RAW_TEXT) { this._consumeRawTextWithTagClose(lowercaseTagName, false); @@ -470,13 +487,20 @@ class _HtmlTokenizer { this._endToken([this._processCarriageReturns(parts.join(''))]); } - private _savePosition(): number[] { return [this.peek, this.index, this.column, this.line]; } + private _savePosition(): number[] { + return [this.peek, this.index, this.column, this.line, this.tokens.length]; + } private _restorePosition(position: number[]): void { this.peek = position[0]; this.index = position[1]; this.column = position[2]; this.line = position[3]; + let nbTokens = position[4]; + if (nbTokens < this.tokens.length) { + // remove any extra tokens + this.tokens = ListWrapper.slice(this.tokens, 0, nbTokens); + } } } @@ -516,3 +540,21 @@ function isAsciiLetter(code: number): boolean { function isAsciiHexDigit(code: number): boolean { return code >= $a && code <= $f || code >= $0 && code <= $9; } + +function mergeTextTokens(srcTokens: HtmlToken[]): HtmlToken[] { + let dstTokens = []; + let lastDstToken: HtmlToken; + for (let i = 0; i < srcTokens.length; i++) { + let token = srcTokens[i]; + if (isPresent(lastDstToken) && lastDstToken.type == HtmlTokenType.TEXT && + token.type == HtmlTokenType.TEXT) { + lastDstToken.parts[0] += token.parts[0]; + lastDstToken.sourceSpan.end = token.sourceSpan.end; + } else { + lastDstToken = token; + dstTokens.push(lastDstToken); + } + } + + return dstTokens; +} diff --git a/modules/angular2/test/compiler/html_lexer_spec.ts b/modules/angular2/test/compiler/html_lexer_spec.ts index 35b0f5676cf3..5d3f74115602 100644 --- a/modules/angular2/test/compiler/html_lexer_spec.ts +++ b/modules/angular2/test/compiler/html_lexer_spec.ts @@ -192,15 +192,6 @@ export function main() { ]); }); - it('should report missing name after <', () => { - expect(tokenizeAndHumanizeErrors('<')) - .toEqual([[HtmlTokenType.TAG_OPEN_START, 'Unexpected character "EOF"', '0:1']]); - }); - - it('should report missing >', () => { - expect(tokenizeAndHumanizeErrors(' { @@ -335,20 +326,6 @@ export function main() { ]); }); - it('should report missing value after =', () => { - expect(tokenizeAndHumanizeErrors(' { - expect(tokenizeAndHumanizeErrors(' { - expect(tokenizeAndHumanizeErrors(' { @@ -448,6 +425,36 @@ export function main() { expect(tokenizeAndHumanizeSourceSpans('a')) .toEqual([[HtmlTokenType.TEXT, 'a'], [HtmlTokenType.EOF, '']]); }); + + it('should allow "<" in text nodes', () => { + expect(tokenizeAndHumanizeParts('{{ a < b ? c : d }}')) + .toEqual([[HtmlTokenType.TEXT, '{{ a < b ? c : d }}'], [HtmlTokenType.EOF]]); + + expect(tokenizeAndHumanizeSourceSpans('

a')) + .toEqual([ + [HtmlTokenType.TAG_OPEN_START, ''], + [HtmlTokenType.TEXT, 'a'], + [HtmlTokenType.EOF, ''], + ]); + }); + + // TODO(vicb): make the lexer aware of Angular expressions + // see https://github.com/angular/angular/issues/5679 + it('should parse valid start tag in interpolation', () => { + expect(tokenizeAndHumanizeParts('{{ a d }}')) + .toEqual([ + [HtmlTokenType.TEXT, '{{ a '], + [HtmlTokenType.TAG_OPEN_START, null, 'b'], + [HtmlTokenType.ATTR_NAME, null, '&&'], + [HtmlTokenType.ATTR_NAME, null, 'c'], + [HtmlTokenType.TAG_OPEN_END], + [HtmlTokenType.TEXT, ' d }}'], + [HtmlTokenType.EOF] + ]); + }); + }); describe('raw text', () => { From e82c6fc50ef6763cde360fd0cddd2e801c9f5e9c Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Sun, 6 Dec 2015 13:12:41 -0800 Subject: [PATCH 3/4] fix(HtmlLexer): tag name must follow "<" without space see http://www.w3.org/TR/html5/syntax.html#tag-open-state --- modules/angular2/src/compiler/html_lexer.ts | 4 +++- modules/angular2/test/compiler/html_lexer_spec.ts | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/angular2/src/compiler/html_lexer.ts b/modules/angular2/src/compiler/html_lexer.ts index e3a725ce9d71..f410fbdb73f4 100644 --- a/modules/angular2/src/compiler/html_lexer.ts +++ b/modules/angular2/src/compiler/html_lexer.ts @@ -378,7 +378,9 @@ class _HtmlTokenizer { let savedPos = this._savePosition(); let lowercaseTagName; try { - this._attemptUntilFn(isNotWhitespace); + if (!isAsciiLetter(this.peek)) { + throw this._createError(unexpectedCharacterErrorMsg(this.peek), this._getLocation()); + } var nameStart = this.index; this._consumeTagOpenStart(start); lowercaseTagName = this.inputLowercase.substring(nameStart, this.index); diff --git a/modules/angular2/test/compiler/html_lexer_spec.ts b/modules/angular2/test/compiler/html_lexer_spec.ts index 5d3f74115602..5e61887eef49 100644 --- a/modules/angular2/test/compiler/html_lexer_spec.ts +++ b/modules/angular2/test/compiler/html_lexer_spec.ts @@ -174,8 +174,8 @@ export function main() { ]); }); - it('should allow whitespace', () => { - expect(tokenizeAndHumanizeParts('< test >')) + it('should allow whitespace after the tag name', () => { + expect(tokenizeAndHumanizeParts('')) .toEqual([ [HtmlTokenType.TAG_OPEN_START, null, 'test'], [HtmlTokenType.TAG_OPEN_END], @@ -438,6 +438,9 @@ export function main() { [HtmlTokenType.TAG_CLOSE, '

'], [HtmlTokenType.EOF, ''], ]); + + expect(tokenizeAndHumanizeParts('< a>')) + .toEqual([[HtmlTokenType.TEXT, '< a>'], [HtmlTokenType.EOF]]); }); // TODO(vicb): make the lexer aware of Angular expressions From ca994e06acae634000896bd1f0dbc21f77ebe30e Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Sat, 5 Dec 2015 00:15:18 -0800 Subject: [PATCH 4/4] fix(HtmlParser): ignore LF immediately following pre, textarea & listing fixes #5630 --- modules/angular2/src/compiler/html_parser.ts | 13 ++++++++++++- modules/angular2/src/compiler/html_tags.ts | 12 +++++++++--- .../angular2/test/compiler/html_parser_spec.ts | 16 ++++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/modules/angular2/src/compiler/html_parser.ts b/modules/angular2/src/compiler/html_parser.ts index fc9c9a234daa..579cbc37cdcb 100644 --- a/modules/angular2/src/compiler/html_parser.ts +++ b/modules/angular2/src/compiler/html_parser.ts @@ -106,7 +106,18 @@ class TreeBuilder { } private _consumeText(token: HtmlToken) { - this._addToParent(new HtmlTextAst(token.parts[0], token.sourceSpan)); + let text = token.parts[0]; + if (text.length > 0 && text[0] == '\n') { + let parent = this._getParentElement(); + if (isPresent(parent) && parent.children.length == 0 && + getHtmlTagDefinition(parent.name).ignoreFirstLf) { + text = text.substring(1); + } + } + + if (text.length > 0) { + this._addToParent(new HtmlTextAst(text, token.sourceSpan)); + } } private _closeVoidElement(): void { diff --git a/modules/angular2/src/compiler/html_tags.ts b/modules/angular2/src/compiler/html_tags.ts index e45ff34d163c..6fc4f23fe5cc 100644 --- a/modules/angular2/src/compiler/html_tags.ts +++ b/modules/angular2/src/compiler/html_tags.ts @@ -279,15 +279,17 @@ export class HtmlTagDefinition { public implicitNamespacePrefix: string; public contentType: HtmlTagContentType; public isVoid: boolean; + public ignoreFirstLf: boolean; constructor({closedByChildren, requiredParents, implicitNamespacePrefix, contentType, - closedByParent, isVoid}: { + closedByParent, isVoid, ignoreFirstLf}: { closedByChildren?: string[], closedByParent?: boolean, requiredParents?: string[], implicitNamespacePrefix?: string, contentType?: HtmlTagContentType, - isVoid?: boolean + isVoid?: boolean, + ignoreFirstLf?: boolean } = {}) { if (isPresent(closedByChildren) && closedByChildren.length > 0) { closedByChildren.forEach(tagName => this.closedByChildren[tagName] = true); @@ -301,6 +303,7 @@ export class HtmlTagDefinition { } this.implicitNamespacePrefix = implicitNamespacePrefix; this.contentType = isPresent(contentType) ? contentType : HtmlTagContentType.PARSABLE_DATA; + this.ignoreFirstLf = normalizeBool(ignoreFirstLf); } requireExtraParent(currentParent: string): boolean { @@ -388,10 +391,13 @@ 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}), + 'pre': new HtmlTagDefinition({ignoreFirstLf: true}), + 'listing': new HtmlTagDefinition({ignoreFirstLf: true}), 'style': new HtmlTagDefinition({contentType: HtmlTagContentType.RAW_TEXT}), 'script': new HtmlTagDefinition({contentType: HtmlTagContentType.RAW_TEXT}), 'title': new HtmlTagDefinition({contentType: HtmlTagContentType.ESCAPABLE_RAW_TEXT}), - 'textarea': new HtmlTagDefinition({contentType: HtmlTagContentType.ESCAPABLE_RAW_TEXT}), + 'textarea': new HtmlTagDefinition( + {contentType: HtmlTagContentType.ESCAPABLE_RAW_TEXT, ignoreFirstLf: true}), }; var DEFAULT_TAG_DEFINITION = new HtmlTagDefinition(); diff --git a/modules/angular2/test/compiler/html_parser_spec.ts b/modules/angular2/test/compiler/html_parser_spec.ts index a9217414f8ee..5094eb3a8da4 100644 --- a/modules/angular2/test/compiler/html_parser_spec.ts +++ b/modules/angular2/test/compiler/html_parser_spec.ts @@ -178,6 +178,22 @@ export function main() { expect(humanizeDom(parser.parse('', 'TestComp'))) .toEqual([[HtmlElementAst, '@math:math', 0]]); }); + + it('should ignore LF immediately after textarea, pre and listing', () => { + expect(humanizeDom(parser.parse( + '

\n

\n\n
\n\n', + 'TestComp'))) + .toEqual([ + [HtmlElementAst, 'p', 0], + [HtmlTextAst, '\n', 1], + [HtmlElementAst, 'textarea', 0], + [HtmlElementAst, 'pre', 0], + [HtmlTextAst, '\n', 1], + [HtmlElementAst, 'listing', 0], + [HtmlTextAst, '\n', 1], + ]); + }); + }); describe('attributes', () => {