Skip to content

Commit

Permalink
Make innerHTML and outerHTML parsing spec compliant
Browse files Browse the repository at this point in the history
This pull request fixes the issue discussed here: #2398 (comment). Before this change, innerHTML and outerHTML would directly manipulate their thisArg, using appendChild() and removeChild(), instead of instead of creating a fragment and inserting it in the DOM at the end of the parsing. Moving to the per-spec architecture fixes various correctness issues.

Other changes:

- Create a generic parser module exposing two methods: parseFragment and parseDocument. This abstracts over the HTML vs. XML distinction.
- Migrate all the internal usages of the "htmltodom" module to use the new parser module.
- Adjust the implementation of Element's innerHTML, outerHTML, and insertAdjacentHTML(), as well as ShadowRoot's innerHTML, to closer match to the spec.
- Update saxes and w3c-xmlserializer to improve XML parsing/serialization.
  • Loading branch information
pmdartus authored and domenic committed Mar 28, 2019
1 parent a0b96af commit 49353e2
Show file tree
Hide file tree
Showing 15 changed files with 367 additions and 314 deletions.
5 changes: 3 additions & 2 deletions lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const MIMEType = require("whatwg-mimetype");
const idlUtils = require("./jsdom/living/generated/utils.js");
const VirtualConsole = require("./jsdom/virtual-console.js");
const Window = require("./jsdom/browser/Window.js");
const { parseIntoDocument } = require("./jsdom/browser/parser");
const { fragmentSerialization } = require("./jsdom/living/domparsing/serialization.js");
const ResourceLoader = require("./jsdom/browser/resources/resource-loader.js");
const NoOpResourceLoader = require("./jsdom/browser/resources/no-op-resource-loader.js");
Expand Down Expand Up @@ -41,8 +42,8 @@ class JSDOM {

options.beforeParse(this[window]._globalProxy);

// TODO NEWAPI: this is still pretty hacky. It's also different than jsdom.jsdom. Does it work? Can it be better?
documentImpl._htmlToDom.appendToDocument(html, documentImpl);
parseIntoDocument(html, documentImpl);

documentImpl.close();
}

Expand Down
191 changes: 0 additions & 191 deletions lib/jsdom/browser/htmltodom.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
"use strict";

const DocumentType = require("../living/generated/DocumentType");
const DocumentFragment = require("../living/generated/DocumentFragment");
const Text = require("../living/generated/Text");
const Comment = require("../living/generated/Comment");
const attributes = require("../living/attributes");
const nodeTypes = require("../living/node-type");
const serializationAdapter = require("../living/domparsing/parse5-adapter-serialization");
const parse5 = require("parse5");

const OpenElementStack = require("parse5/lib/parser/open-element-stack");
const DocumentType = require("../../living/generated/DocumentType");
const DocumentFragment = require("../../living/generated/DocumentFragment");
const Text = require("../../living/generated/Text");
const Comment = require("../../living/generated/Comment");

const attributes = require("../../living/attributes");
const nodeTypes = require("../../living/node-type");

const serializationAdapter = require("../../living/domparsing/parse5-adapter-serialization");

const OpenElementStack = require("parse5/lib/parser/open-element-stack");
const OpenElementStackOriginalPop = OpenElementStack.prototype.pop;
const OpenElementStackOriginalPush = OpenElementStack.prototype.push;

module.exports = class JSDOMParse5Adapter {
class JSDOMParse5Adapter {
constructor(documentImpl) {
this._documentImpl = documentImpl;

Expand Down Expand Up @@ -82,11 +85,11 @@ module.exports = class JSDOMParse5Adapter {
}

appendChild(parentNode, newNode) {
parentNode.appendChild(newNode);
parentNode._append(newNode);
}

insertBefore(parentNode, newNode, referenceNode) {
parentNode.insertBefore(newNode, referenceNode);
parentNode._insert(newNode, referenceNode);
}

setTemplateContent(templateElement, contentFragment) {
Expand All @@ -109,7 +112,7 @@ module.exports = class JSDOMParse5Adapter {
const ownerDocument = this._ownerDocument();
const documentType = DocumentType.createImpl([], { name, publicId, systemId, ownerDocument });

document.appendChild(documentType);
document._append(documentType);
}

setDocumentMode(document, mode) {
Expand All @@ -128,7 +131,7 @@ module.exports = class JSDOMParse5Adapter {
} else {
const ownerDocument = this._ownerDocument();
const textNode = Text.createImpl([], { data: text, ownerDocument });
parentNode.appendChild(textNode);
parentNode._append(textNode);
}
}

Expand All @@ -139,7 +142,7 @@ module.exports = class JSDOMParse5Adapter {
} else {
const ownerDocument = this._ownerDocument();
const textNode = Text.createImpl([], { data: text, ownerDocument });
parentNode.insertBefore(textNode, referenceNode);
parentNode._append(textNode, referenceNode);
}
}

Expand All @@ -149,6 +152,30 @@ module.exports = class JSDOMParse5Adapter {
attributes.setAttributeValue(element, attr.name, attr.value, prefix, attr.namespace);
}
}
};
}

// Assign shared adapters with serializer.
Object.assign(JSDOMParse5Adapter.prototype, serializationAdapter);

Object.assign(module.exports.prototype, serializationAdapter);
function parseFragment(markup, contextElement) {
const ownerDocument = contextElement._ownerDocument;

const config = Object.assign({}, ownerDocument._parseOptions, {
treeAdapter: new JSDOMParse5Adapter(ownerDocument)
});

return parse5.parseFragment(contextElement, markup, config);
}

function parseIntoDocument(markup, ownerDocument) {
const config = Object.assign({}, ownerDocument._parseOptions, {
treeAdapter: new JSDOMParse5Adapter(ownerDocument)
});

return parse5.parse(markup, config);
}

module.exports = {
parseFragment,
parseIntoDocument
};
37 changes: 37 additions & 0 deletions lib/jsdom/browser/parser/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
"use strict";

const xmlParser = require("./xml");
const htmlParser = require("./html");

// https://w3c.github.io/DOM-Parsing/#dfn-fragment-parsing-algorithm
function parseFragment(markup, contextElement) {
const { _parsingMode } = contextElement._ownerDocument;

let parseAlgorithm;
if (_parsingMode === "html") {
parseAlgorithm = htmlParser.parseFragment;
} else if (_parsingMode === "xml") {
parseAlgorithm = xmlParser.parseFragment;
}

// Note: HTML and XML fragment parsing algorithm already return a document fragments; no need to do steps 3 and 4
return parseAlgorithm(markup, contextElement);
}

function parseIntoDocument(markup, ownerDocument) {
const { _parsingMode } = ownerDocument;

let parseAlgorithm;
if (_parsingMode === "html") {
parseAlgorithm = htmlParser.parseIntoDocument;
} else if (_parsingMode === "xml") {
parseAlgorithm = xmlParser.parseIntoDocument;
}

return parseAlgorithm(markup, ownerDocument);
}

module.exports = {
parseFragment,
parseIntoDocument
};
Loading

0 comments on commit 49353e2

Please sign in to comment.