Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Allow omitted optional close tags in HTMLSimpleDOM (#12057)
Browse files Browse the repository at this point in the history
* Allow omitted optional close tags in HTMLSimpleDOM

fixes #7257
This commit also updates the openImpliesClose list.

* Fix TypeError after typing at end of html document

This fixes `TypeError: Cannot read property 'mark' of null`

* Make shure Tokenizer gets to the end

and does not skip text nodes at the end.

* Fix preferParent in HTMLInstrumentation

* Allow dt at end of dl

...even though it's not actually valid. Change requested by @petetnt
jscissr authored and petetnt committed Oct 25, 2016

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent ab1396b commit 1cfb24f
Showing 5 changed files with 112 additions and 44 deletions.
15 changes: 5 additions & 10 deletions src/language/HTMLInstrumentation.js
Original file line number Diff line number Diff line change
@@ -142,21 +142,16 @@ define(function (require, exports, module) {
}

// The mark with the latest start is the innermost one.
match = marks[marks.length - 1];
match = marks.pop();
if (preferParent) {
// If the match is exactly at the edge of the range and preferParent is set,
// we want to pop upwards.
if (_posEq(match.range.from, pos) || _posEq(match.range.to, pos)) {
if (marks.length > 1) {
match = marks[marks.length - 2];
} else {
// We must be outside the root, so there's no containing tag.
match = null;
}
// we want to pop upwards. If pos is exactly between two marks, we need to pop upwards twice.
while (match && (_posEq(match.range.from, pos) || _posEq(match.range.to, pos))) {
match = marks.pop();
}
}

return match.mark;
return match && match.mark;
}

/**
79 changes: 53 additions & 26 deletions src/language/HTMLSimpleDOM.js
Original file line number Diff line number Diff line change
@@ -48,10 +48,14 @@ define(function (require, exports, module) {
article : { p: true },
aside : { p: true },
blockquote : { p: true },
colgroup: { caption: true },
details : { p: true },
dir : { p: true },
div : { p: true },
dl : { p: true },
fieldset: { p: true },
figcaption: { p: true },
figure : { p: true },
footer : { p: true },
form : { p: true },
h1 : { p: true },
@@ -72,18 +76,49 @@ define(function (require, exports, module) {
section : { p: true },
table : { p: true },
ul : { p: true },
rt : { rp: true, rt: true },
rp : { rp: true, rt: true },
rb : { rb: true, rt: true, rtc: true, rp: true },
rp : { rb: true, rt: true, rp: true },
rt : { rb: true, rt: true, rp: true },
rtc : { rb: true, rt: true, rtc: true, rp: true },
optgroup: { optgroup: true, option: true },
option : { option: true },
tbody : { thead: true, tbody: true, tfoot: true },
tfoot : { tbody: true },
tr : { tr: true, th: true, td: true },
tbody : { caption: true, colgroup: true, thead: true, tbody: true, tfoot: true, },
tfoot : { caption: true, colgroup: true, thead: true, tbody: true },
thead : { caption: true, colgroup: true },
tr : { tr: true, th: true, td: true, caption: true },
th : { th: true, td: true },
td : { thead: true, th: true, td: true },
body : { head: true, link: true, script: true }
td : { th: true, td: true },
body : { head: true }
};

/**
* A list of elements which are automatically closed when their parent is closed:
* http://www.w3.org/html/wg/drafts/html/master/syntax.html#optional-tags
*/
var optionalClose = {
html: true,
body: true,
li: true,
dd: true,
dt: true, // This is not actually correct, but showing a syntax error is not helpful
p: true,
rb: true,
rt: true,
rtc: true,
rp: true,
optgroup: true,
option: true,
colgroup: true,
caption: true,
tbody: true,
tfoot: true,
tr: true,
td: true,
th: true
};

// TODO: handle optional start tags

/**
* A list of tags that are self-closing (do not contain other elements).
* Mostly taken from http://www.w3.org/html/wg/drafts/html/master/syntax.html#void-elements
@@ -382,13 +417,6 @@ define(function (require, exports, module) {
break;
}
}
if (strict && i !== stack.length - 1) {
// If we're in strict mode, treat unbalanced tags as invalid.
PerfUtils.finalizeMeasurement(timerBuildFull);
PerfUtils.addMeasurement(timerBuildPart);
this._logError(token);
return null;
}
if (i >= 0) {
do {
// For all tags we're implicitly closing (before we hit the matching tag), we want the
@@ -399,6 +427,13 @@ define(function (require, exports, module) {
if (stack.length === i + 1) {
closeTag(token.end + 1, _offsetPos(token.endPos, 1));
} else {
if (strict && !optionalClose.hasOwnProperty(stack[stack.length - 1].tag)) {
// If we're in strict mode, treat unbalanced tags as invalid.
PerfUtils.finalizeMeasurement(timerBuildFull);
PerfUtils.addMeasurement(timerBuildPart);
this._logError(token);
return null;
}
closeTag(token.start - 2, _offsetPos(token.startPos, -2));
}
} while (stack.length > i);
@@ -447,24 +482,16 @@ define(function (require, exports, module) {
lastIndex = token.end;
}

// If we have any tags hanging open (e.g. html or body), fail the parse if we're in strict mode,
// If we have any tags hanging open, fail the parse if we're in strict mode,
// otherwise close them at the end of the document.
if (stack.length) {
if (strict) {
while (stack.length) {
if (strict && !optionalClose.hasOwnProperty(stack[stack.length - 1].tag)) {
PerfUtils.finalizeMeasurement(timerBuildFull);
PerfUtils.addMeasurement(timerBuildPart);
this._logError(token);
return null;
} else {
// Manually compute the position of the end of the text (we can't rely on the
// tokenizer for this since it may not get to the very end)
// TODO: should probably make the tokenizer get to the end...
var lines = this.text.split("\n"),
lastPos = {line: lines.length - 1, ch: lines[lines.length - 1].length};
while (stack.length) {
closeTag(this.text.length, lastPos);
}
}
closeTag(this.text.length, this.t._indexPos);
}

var dom = lastClosedTag;
11 changes: 8 additions & 3 deletions src/language/HTMLTokenizer.js
Original file line number Diff line number Diff line change
@@ -678,9 +678,14 @@ define(function (require, exports, module) {
this._index++;
}

if (this._index === this._buffer.length && this._state !== TEXT) {
// We hit EOF in the middle of processing something else.
this._emitSpecialToken("error");
if (!this._token) {
if (this._state !== TEXT) {
// We hit EOF in the middle of processing something else.
this._emitSpecialToken("error");
} else {
this._emitTokenIfNonempty("text");
this._startSection();
}
}
return this._token;
};
36 changes: 31 additions & 5 deletions test/spec/HTMLInstrumentation-test.js
Original file line number Diff line number Diff line change
@@ -1431,7 +1431,7 @@ define(function (require, exports, module) {
});

it("should handle deleting of a non-empty tag character-by-character", function () {
setupEditor("<div><p>deleteme</p>{{0}}</div>", true);
setupEditor("<div><b>deleteme</b>{{0}}</div>", true);
runs(function () {
var previousDOM = HTMLSimpleDOM.build(editor.document.getText()),
pTagID = previousDOM.children[0].tagID,
@@ -1451,6 +1451,22 @@ define(function (require, exports, module) {
});
});

it("should handle deleting of a single character exactly between two elements", function () {
setupEditor("<p><br>X{{0}}<br></p>", true);
runs(function () {
var previousDOM = HTMLSimpleDOM.build(editor.document.getText()),
pTagID = previousDOM.tagID,
br1TagID = previousDOM.children[0].tagID,
br2TagID = previousDOM.children[2].tagID;

HTMLInstrumentation._markTextFromDOM(editor, previousDOM);

deleteAndExpect(editor, previousDOM, offsets[0], 1, [
[{type: 'textDelete', parentID: pTagID, afterID: br1TagID, beforeID: br2TagID}]
]);
});
});

it("should handle typing of a new attribute character-by-character", function () {
setupEditor("<p{{0}}>some text</p>", true);
runs(function () {
@@ -1607,10 +1623,10 @@ define(function (require, exports, module) {
expect(previousDOM).toBe(null);

// Type the opening tag--should be invalid all the way
result = typeAndExpect(editor, previousDOM, {line: 0, ch: 0}, "<html></html");
result = typeAndExpect(editor, previousDOM, {line: 0, ch: 0}, "<html");
expect(result.finalInvalid).toBe(true);

// Finally become valid by closing the end tag. Note that this elementInsert
// Finally become valid by closing the start tag. Note that this elementInsert
// should be treated specially by RemoteFunctions not to actually insert the
// element, but just copy its ID to the autocreated HTML element.
result = typeAndExpect(editor, result.finalDOM, result.finalPos, ">", [
@@ -1675,10 +1691,10 @@ define(function (require, exports, module) {
HTMLInstrumentation._markTextFromDOM(editor, previousDOM);

// Type the opening tag--should be invalid all the way
result = typeAndExpect(editor, previousDOM, offsets[0], "<body></body");
result = typeAndExpect(editor, previousDOM, offsets[0], "<body");
expect(result.finalInvalid).toBe(true);

// Finally become valid by closing the end tag. Note that this elementInsert
// Finally become valid by closing the start tag. Note that this elementInsert
// should be treated specially by RemoteFunctions not to actually insert the
// element, but just copy its ID to the autocreated HTML element.
result = typeAndExpect(editor, result.finalDOM, result.finalPos, ">", [
@@ -1698,6 +1714,16 @@ define(function (require, exports, module) {
});
});

it("should handle adding a space after </html>", function () {
setupEditor("<html></html>", true);
runs(function () {
doEditTest(editor.document.getText(), function (editor, previousDOM) {
editor.document.replaceRange(" ", {line: 0, ch: 13});
}, function (result, previousDOM, incremental) {
}, true);
});
});

it("should represent simple new tag insert", function () {
setupEditor(WellFormedDoc);
runs(function () {
15 changes: 15 additions & 0 deletions test/spec/HTMLSimpleDOM-test.js
Original file line number Diff line number Diff line change
@@ -71,6 +71,21 @@ define(function (require, exports, module) {
expect(result.children[1].tag).toBe("h1");
});

it("should parse a document with an implied-close tag followed by an actual close tag", function () {
var result = build("<div><p>unclosed para</div>", true);
expect(result).toBeTruthy();
expect(result.tag).toBe("div");
expect(result.children[0].tag).toBe("p");
});

it("should parse a document with an implied-close tag at the end of the document", function () {
var result = build("<body><p>hello", true);
expect(result).toBeTruthy();
expect(result.tag).toBe("body");
expect(result.children[0].tag).toBe("p");
expect(result.children[0].children[0].content).toBe("hello");
});

it("should return null for an unclosed non-void/non-implied-close tag", function () {
var errors = [{
token : {

0 comments on commit 1cfb24f

Please sign in to comment.