From f65750a4c70bc5b0ed84ab6c404570b60e4e4218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20B=C3=B6hm?= <188768+fb55@users.noreply.github.com> Date: Sun, 27 Feb 2022 09:50:02 +0000 Subject: [PATCH 1/4] refactor(parser): Consume tokenizer events --- packages/parse5/lib/parser/index.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/parse5/lib/parser/index.ts b/packages/parse5/lib/parser/index.ts index ab989bfdd..4d621712e 100644 --- a/packages/parse5/lib/parser/index.ts +++ b/packages/parse5/lib/parser/index.ts @@ -974,6 +974,13 @@ export class Parser implements TokenHandler { } else { this._startTagOutsideForeignContent(token); } + + // FIXME: This can currently fire unexpectedly. + if (token.selfClosing && !token.ackSelfClosing) { + this._err(token, ERR.nonVoidHtmlElementStartTagWithTrailingSolidus); + // Prevent this error from being shown again. + token.ackSelfClosing = true; + } } _startTagOutsideForeignContent(token: TagToken): void { switch (this.insertionMode) { From c163dea5a7e48e75fdb460b75650917cfbc56255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20B=C3=B6hm?= <188768+fb55@users.noreply.github.com> Date: Sun, 27 Feb 2022 16:26:55 +0000 Subject: [PATCH 2/4] Fix false positives for nested start tag calls --- packages/parse5/lib/parser/index.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/parse5/lib/parser/index.ts b/packages/parse5/lib/parser/index.ts index 4d621712e..ab989bfdd 100644 --- a/packages/parse5/lib/parser/index.ts +++ b/packages/parse5/lib/parser/index.ts @@ -974,13 +974,6 @@ export class Parser implements TokenHandler { } else { this._startTagOutsideForeignContent(token); } - - // FIXME: This can currently fire unexpectedly. - if (token.selfClosing && !token.ackSelfClosing) { - this._err(token, ERR.nonVoidHtmlElementStartTagWithTrailingSolidus); - // Prevent this error from being shown again. - token.ackSelfClosing = true; - } } _startTagOutsideForeignContent(token: TagToken): void { switch (this.insertionMode) { From b0842c2728f734e73409a1b509083fc3f9b1f7fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20B=C3=B6hm?= <188768+fb55@users.noreply.github.com> Date: Sun, 27 Feb 2022 18:44:25 +0000 Subject: [PATCH 3/4] refactor: Use handler for `OpenElementStack` Now that the tokenizer is using the delegate pattern, it only makes sense to update the `OpenElementStack` to follow the same pattern. --- packages/parse5/lib/parser/index.ts | 15 ++--- .../lib/parser/open-element-stack.test.ts | 59 ++++++++++--------- .../parse5/lib/parser/open-element-stack.ts | 22 +++---- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/packages/parse5/lib/parser/index.ts b/packages/parse5/lib/parser/index.ts index ab989bfdd..f52b94486 100644 --- a/packages/parse5/lib/parser/index.ts +++ b/packages/parse5/lib/parser/index.ts @@ -1,5 +1,5 @@ import { TokenHandler, Tokenizer, TokenizerMode } from '../tokenizer/index.js'; -import { OpenElementStack } from './open-element-stack.js'; +import { OpenElementStack, StackHandler } from './open-element-stack.js'; import { FormattingElementList, ElementEntry, EntryType } from './formatting-element-list.js'; import * as defaultTreeAdapter from '../tree-adapters/default.js'; import * as doctype from '../common/doctype.js'; @@ -119,7 +119,7 @@ const defaultParserOptions = { }; //Parser -export class Parser implements TokenHandler { +export class Parser implements TokenHandler, StackHandler { treeAdapter: TreeAdapter; onParseError: ParserErrorHandler | null; private currentToken: Token | null = null; @@ -152,12 +152,7 @@ export class Parser implements TokenHandler { this.fragmentContextID = fragmentContext ? getTagID(this.treeAdapter.getTagName(fragmentContext)) : $.UNKNOWN; this._setContextModes(fragmentContext ?? this.document, this.fragmentContextID); - this.openElements = new OpenElementStack( - this.document, - this.treeAdapter, - this.onItemPush.bind(this), - this.onItemPop.bind(this) - ); + this.openElements = new OpenElementStack(this.document, this.treeAdapter, this); } // API @@ -289,12 +284,12 @@ export class Parser implements TokenHandler { } //Text parsing - private onItemPush(node: T['parentNode'], tid: number, isTop: boolean): void { + onItemPush(node: T['parentNode'], tid: number, isTop: boolean): void { this.treeAdapter.onItemPush?.(node); if (isTop && this.openElements.stackTop > 0) this._setContextModes(node, tid); } - private onItemPop(node: T['parentNode'], isTop: boolean): void { + onItemPop(node: T['parentNode'], isTop: boolean): void { if (this.options.sourceCodeLocationInfo) { this._setEndLocation(node, this.currentToken!); } diff --git a/packages/parse5/lib/parser/open-element-stack.test.ts b/packages/parse5/lib/parser/open-element-stack.test.ts index fb0842806..ba0b7d9b9 100644 --- a/packages/parse5/lib/parser/open-element-stack.test.ts +++ b/packages/parse5/lib/parser/open-element-stack.test.ts @@ -8,6 +8,11 @@ function ignore(): void { /* Ignore */ } +const stackHandler = { + onItemPop: ignore, + onItemPush: ignore, +}; + generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { function createElement(tagName: string, namespaceURI = NS.HTML): TreeAdapterTypeMap['element'] { return treeAdapter.createElement(tagName, namespaceURI, []); @@ -17,7 +22,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { const document = treeAdapter.createDocument(); const element1 = createElement('#element1', NS.XLINK); const element2 = createElement('#element2', NS.SVG); - const stack = new OpenElementStack(document, treeAdapter, ignore, ignore); + const stack = new OpenElementStack(document, treeAdapter, stackHandler); assert.strictEqual(stack.current, document); assert.strictEqual(stack.stackTop, -1); @@ -33,7 +38,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Pop element', () => { const element = createElement('#element', NS.XLINK); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(element, $.UNKNOWN); stack.push(createElement('#element2', NS.XML), $.UNKNOWN); @@ -50,7 +55,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Replace element', () => { const element = createElement('#element', NS.MATHML); const newElement = createElement('#newElement', NS.SVG); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement('#element2', NS.XML), $.UNKNOWN); stack.push(element, $.UNKNOWN); @@ -63,7 +68,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { const element1 = createElement('#element1', NS.XLINK); const element2 = createElement('#element2', NS.SVG); const element3 = createElement('#element3', NS.XML); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(element1, $.UNKNOWN); stack.push(element2, $.UNKNOWN); @@ -79,7 +84,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Pop elements until popped with given tagName', () => { const element1 = createElement(TN.ASIDE); const element2 = createElement(TN.MAIN); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(element2, $.MAIN); stack.push(element2, $.MAIN); @@ -100,7 +105,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Pop elements until given element popped', () => { const element1 = createElement('#element1'); const element2 = createElement('#element2'); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(element2, $.UNKNOWN); stack.push(element2, $.UNKNOWN); @@ -121,7 +126,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Pop elements until numbered header popped', () => { const element1 = createElement(TN.H3); const element2 = createElement(TN.DIV); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(element2, $.DIV); stack.push(element2, $.DIV); @@ -141,7 +146,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Pop all up to element', () => { const htmlElement = createElement(TN.HTML); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(htmlElement, $.HTML); stack.push('#element1', $.UNKNOWN); @@ -155,7 +160,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { const htmlElement = createElement(TN.HTML); const tableElement = createElement(TN.TABLE); const divElement = createElement(TN.DIV); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(htmlElement, $.HTML); stack.push(divElement, $.DIV); @@ -178,7 +183,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { const htmlElement = createElement(TN.HTML); const theadElement = createElement(TN.THEAD); const divElement = createElement(TN.DIV); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(htmlElement, $.HTML); stack.push(divElement, $.DIV); @@ -201,7 +206,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { const htmlElement = createElement(TN.HTML); const trElement = createElement(TN.TR); const divElement = createElement(TN.DIV); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(htmlElement, $.HTML); stack.push(divElement, $.DIV); @@ -222,7 +227,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Remove element', () => { const element = createElement('#element'); - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(element, $.UNKNOWN); stack.push(createElement('element1'), $.UNKNOWN); @@ -239,20 +244,20 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { test('Try peek properly nested element', () => { const bodyElement = createElement(TN.BODY); - let stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + let stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(bodyElement, $.BODY); stack.push(createElement(TN.DIV), $.DIV); assert.strictEqual(stack.tryPeekProperlyNestedBodyElement(), bodyElement); - stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); assert.ok(!stack.tryPeekProperlyNestedBodyElement()); }); test('Is root element current', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); assert.ok(stack.isRootHtmlElementCurrent()); @@ -262,7 +267,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Get common ancestor', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); const element = createElement('#element'); const ancestor = createElement('#ancestor'); @@ -282,7 +287,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Contains element', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); const element = createElement('#element'); stack.push(createElement('#someElement'), $.UNKNOWN); @@ -293,7 +298,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Has element in scope', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.DIV), $.DIV); @@ -310,7 +315,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Has numbered header in scope', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.DIV), $.DIV); @@ -330,7 +335,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Has element in list item scope', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.DIV), $.DIV); @@ -346,7 +351,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Has element in button scope', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.DIV), $.DIV); @@ -362,7 +367,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Has element in table scope', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.DIV), $.DIV); @@ -379,7 +384,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Has table body context in table scope', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.DIV), $.DIV); @@ -399,7 +404,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Has element in select scope', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.DIV), $.DIV); @@ -414,7 +419,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Generate implied end tags', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.LI), $.LI); @@ -430,7 +435,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Generate implied end tags with exclusion', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.LI), $.LI); @@ -446,7 +451,7 @@ generateTestsForEachTreeAdapter('open-element-stack', (treeAdapter) => { }); test('Template count', () => { - const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, ignore, ignore); + const stack = new OpenElementStack(treeAdapter.createDocument(), treeAdapter, stackHandler); stack.push(createElement(TN.HTML), $.HTML); stack.push(createElement(TN.TEMPLATE, NS.MATHML), $.TEMPLATE); diff --git a/packages/parse5/lib/parser/open-element-stack.ts b/packages/parse5/lib/parser/open-element-stack.ts index 02054eebd..ea75b2103 100644 --- a/packages/parse5/lib/parser/open-element-stack.ts +++ b/packages/parse5/lib/parser/open-element-stack.ts @@ -41,6 +41,11 @@ const TABLE_BODY_CONTEXT = [$.TBODY, $.TFOOT, $.THEAD, $.TEMPLATE, $.HTML]; const TABLE_CONTEXT = [$.TABLE, $.TEMPLATE, $.HTML]; const TABLE_CELLS = [$.TD, $.TH]; +export interface StackHandler { + onItemPush: (node: T['parentNode'], tid: number, isTop: boolean) => void; + onItemPop: (node: T['parentNode'], isTop: boolean) => void; +} + //Stack of open elements export class OpenElementStack { items: T['parentNode'][] = []; @@ -55,12 +60,7 @@ export class OpenElementStack { return this._isInTemplate() ? this.treeAdapter.getTemplateContent(this.current) : this.current; } - constructor( - document: T['document'], - private treeAdapter: TreeAdapter, - private onItemPush: (node: T['parentNode'], tid: number, isTop: boolean) => void, - private onItemPop: (node: T['parentNode'], isTop: boolean) => void - ) { + constructor(document: T['document'], private treeAdapter: TreeAdapter, private handler: StackHandler) { this.current = document; } @@ -92,7 +92,7 @@ export class OpenElementStack { this.tmplCount++; } - this.onItemPush(element, tagID, true); + this.handler.onItemPush(element, tagID, true); } pop(): void { @@ -104,7 +104,7 @@ export class OpenElementStack { this.stackTop--; this._updateCurrentElement(); - this.onItemPop(popped, true); + this.handler.onItemPop(popped, true); } replace(oldElement: T['element'], newElement: T['element']): void { @@ -128,7 +128,7 @@ export class OpenElementStack { this._updateCurrentElement(); } - this.onItemPush(this.current, this.currentTagId, insertionIdx === this.stackTop); + this.handler.onItemPush(this.current, this.currentTagId, insertionIdx === this.stackTop); } popUntilTagNamePopped(tagName: $): void { @@ -152,7 +152,7 @@ export class OpenElementStack { this.stackTop--; this._updateCurrentElement(); - this.onItemPop(popped, this.stackTop < idx); + this.handler.onItemPop(popped, this.stackTop < idx); } } @@ -218,7 +218,7 @@ export class OpenElementStack { this.tagIDs.splice(idx, 1); this.stackTop--; this._updateCurrentElement(); - this.onItemPop(element, false); + this.handler.onItemPop(element, false); } } } From 43188a1493fc75175cc8003f1a64f8ca06355fee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20B=C3=B6hm?= <188768+fb55@users.noreply.github.com> Date: Wed, 2 Mar 2022 11:06:54 +0000 Subject: [PATCH 4/4] Fix comment --- packages/parse5/lib/parser/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/parse5/lib/parser/index.ts b/packages/parse5/lib/parser/index.ts index f52b94486..2edb60040 100644 --- a/packages/parse5/lib/parser/index.ts +++ b/packages/parse5/lib/parser/index.ts @@ -283,7 +283,7 @@ export class Parser implements TokenHandler, Stack writeCallback?.(); } - //Text parsing + //Stack events onItemPush(node: T['parentNode'], tid: number, isTop: boolean): void { this.treeAdapter.onItemPush?.(node); if (isTop && this.openElements.stackTop > 0) this._setContextModes(node, tid);