Skip to content

GitLab

T
tangled
Commit 18a1a860 authored by tkent's avatar tkent Committed by Commit bot
Browse files
Options

Editing: Tidy up HTML document structure before execCommand(). 

A crash of crbug.com/537815 was due to an editable <p> as document.documentElement.
Supporting such invalid HTML structure is costly.  So we correct such HTML structure
before executing execCommands.

- We show a console warning if the autocorrection happens.
- After the autocorrection, document.write() doesn't work because the document tree
is complete.

BUG=537815

Review URL: https://codereview.chromium.org/1637323002

Cr-Commit-Position: refs/heads/master@{#371774}
parent 60ac8700
Hide whitespace changes
Inline Side-by-side
Showing with 86 additions and 7 deletions
third_party/WebKit/LayoutTests/editing/editability/empty-document-delete-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
This test ensures WebKit does not crash when executing Delete in an empty document. This test ensures WebKit does not crash when executing Delete in an empty document.
PASS PASS
third_party/WebKit/LayoutTests/editing/editability/empty-document-delete.html
View file @ 18a1a860
...@@ -10,7 +10,7 @@ function runTest() { ...@@ -10,7 +10,7 @@ function runTest() {
document.open(); document.open();
window.getSelection().addRange(document.createRange()); window.getSelection().addRange(document.createRange());
document.execCommand("Delete", false, null); document.execCommand("Delete", false, null);
document.writeln('This test ensures WebKit does not crash when executing Delete in an empty document.<br><br>PASS'); document.body.innerHTML = 'This test ensures WebKit does not crash when executing Delete in an empty document.<br><br>PASS';
} }
</script> </script>
</head> </head>
......
third_party/WebKit/LayoutTests/editing/editability/empty-document-justify-right-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
This test ensures WebKit does not crash when executing JustifyRight in an empty document. This test ensures WebKit does not crash when executing JustifyRight in an empty document.
PASS PASS
third_party/WebKit/LayoutTests/editing/editability/empty-document-justify-right.html
View file @ 18a1a860
...@@ -10,7 +10,7 @@ function runTest() { ...@@ -10,7 +10,7 @@ function runTest() {
document.open(); document.open();
window.getSelection().addRange(document.createRange()); window.getSelection().addRange(document.createRange());
document.execCommand("JustifyRight", false, null); document.execCommand("JustifyRight", false, null);
document.writeln('This test ensures WebKit does not crash when executing JustifyRight in an empty document.<br><br>PASS'); document.body.innerHTML = 'This test ensures WebKit does not crash when executing JustifyRight in an empty document.<br><br>PASS';
} }
</script> </script>
</head> </head>
......
third_party/WebKit/LayoutTests/editing/editability/empty-document-stylewithcss-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
This test ensures WebKit executes StyleWithCSS properly even in an empty document. This test ensures WebKit executes StyleWithCSS properly even in an empty document.
First negation:PASS First negation:PASS
Second negation:PASS Second negation:PASS
......
third_party/WebKit/LayoutTests/editing/execCommand/append-node-under-document-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
This test ensures WebKit does not crash when replacing contents in a document whose the only content is a nested iframes. This test ensures WebKit does not crash when replacing contents in a document whose the only content is a nested iframes.
PASS. PASS.
third_party/WebKit/LayoutTests/editing/execCommand/delete-empty-container-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
Deleting an empty selection should have no effect. Deleting an empty selection should have no effect.
third_party/WebKit/LayoutTests/editing/execCommand/insert-html-to-document-element-crash-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
PASS if Blink doesn't crash. PASS if Blink doesn't crash.
third_party/WebKit/LayoutTests/editing/execCommand/insert-line-break-onload-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
PASS PASS
third_party/WebKit/LayoutTests/editing/execCommand/insert-line-break-onload.html
View file @ 18a1a860
...@@ -11,10 +11,10 @@ function go() { ...@@ -11,10 +11,10 @@ function go() {
var result = "PASS"; var result = "PASS";
if (window.testRunner) { if (window.testRunner) {
document.write("<html>" + result + "<html>"); document.body.innerHTML = result;
testRunner.dumpAsText(); testRunner.dumpAsText();
} else } else
document.write("<html>" + result + "<html>"); document.body.innerHTML = result;
} }
</script> </script>
</body> </body>
......
third_party/WebKit/LayoutTests/editing/inserting/insert-paragraph-separator-crash2.html 0 → 100644
View file @ 18a1a860
<!DOCTYPE html>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script>
test(function() {
var p = document.createElement('p');
document.replaceChild(p, document.documentElement);
p.innerHTML = '<form>\n<p>\n<ruby>\n<rb>\n</rb></ruby></p><table>\n</table></form>\n<ruby>\n<p>\n</p><table></table></ruby>';
var s = document.querySelector('rb').firstChild;
var e = document.querySelector('table');
var ol = document.createElement('ol');
ol.innerHTML = '\n<li>\n\n</li>\n';
e.insertBefore(ol, e.firstChild);
var r = document.createRange();
r.setStart(s, 0);
r.setEnd(e, 1);
window.getSelection().removeAllRanges();
window.getSelection().addRange(r);
document.designMode = 'on';
document.execCommand('InsertParagraph');
}, 'InsertParagraph command should not crash with editable P root element.');
</script>
third_party/WebKit/LayoutTests/editing/selection/deleteFromDocument-after-document-open-crash-expected.txt
View file @ 18a1a860
CONSOLE WARNING: document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically.
This calling deleteFromDocument after document.open. WebKit should not crash. This calling deleteFromDocument after document.open. WebKit should not crash.
PASS PASS
third_party/WebKit/LayoutTests/editing/selection/deleteFromDocument-after-document-open-crash.html
View file @ 18a1a860
...@@ -17,8 +17,7 @@ function run() { ...@@ -17,8 +17,7 @@ function run() {
selection.deleteFromDocument(); selection.deleteFromDocument();
document.write('This calling deleteFromDocument after document.open. WebKit should not crash.<br>'); document.body.innerHTML = 'This calling deleteFromDocument after document.open. WebKit should not crash.<br>PASS';
document.write('PASS<br>');
} }
</script> </script>
</head> </head>
......
third_party/WebKit/Source/core/dom/Document.cpp
View file @ 18a1a860
...@@ -4455,6 +4455,7 @@ bool Document::execCommand(const String& commandName, bool, const String& value, ...@@ -4455,6 +4455,7 @@ bool Document::execCommand(const String& commandName, bool, const String& value,
// Postpone DOM mutation events, which can execute scripts and change // Postpone DOM mutation events, which can execute scripts and change
// DOM tree against implementation assumption. // DOM tree against implementation assumption.
EventQueueScope eventQueueScope; EventQueueScope eventQueueScope;
Editor::tidyUpHTMLStructure(*this);
Editor::Command editorCommand = command(this, commandName); Editor::Command editorCommand = command(this, commandName);
Platform::current()->histogramSparse("WebCore.Document.execCommand", editorCommand.idForHistogram()); Platform::current()->histogramSparse("WebCore.Document.execCommand", editorCommand.idForHistogram());
return editorCommand.execute(value); return editorCommand.execute(value);
......
third_party/WebKit/Source/core/editing/Editor.cpp
View file @ 18a1a860
...@@ -38,6 +38,7 @@ ...@@ -38,6 +38,7 @@
#include "core/css/StylePropertySet.h" #include "core/css/StylePropertySet.h"
#include "core/dom/AXObjectCache.h" #include "core/dom/AXObjectCache.h"
#include "core/dom/DocumentFragment.h" #include "core/dom/DocumentFragment.h"
#include "core/dom/ElementTraversal.h"
#include "core/dom/NodeTraversal.h" #include "core/dom/NodeTraversal.h"
#include "core/dom/ParserContentPolicy.h" #include "core/dom/ParserContentPolicy.h"
#include "core/dom/Text.h" #include "core/dom/Text.h"
...@@ -69,12 +70,15 @@ ...@@ -69,12 +70,15 @@
#include "core/frame/LocalFrame.h" #include "core/frame/LocalFrame.h"
#include "core/frame/Settings.h" #include "core/frame/Settings.h"
#include "core/frame/UseCounter.h" #include "core/frame/UseCounter.h"
#include "core/html/HTMLBodyElement.h"
#include "core/html/HTMLCanvasElement.h" #include "core/html/HTMLCanvasElement.h"
#include "core/html/HTMLHtmlElement.h"
#include "core/html/HTMLImageElement.h" #include "core/html/HTMLImageElement.h"
#include "core/html/HTMLInputElement.h" #include "core/html/HTMLInputElement.h"
#include "core/html/HTMLTextAreaElement.h" #include "core/html/HTMLTextAreaElement.h"
#include "core/html/parser/HTMLParserIdioms.h" #include "core/html/parser/HTMLParserIdioms.h"
#include "core/input/EventHandler.h" #include "core/input/EventHandler.h"
#include "core/inspector/ConsoleMessage.h"
#include "core/layout/HitTestResult.h" #include "core/layout/HitTestResult.h"
#include "core/layout/LayoutImage.h" #include "core/layout/LayoutImage.h"
#include "core/loader/EmptyClients.h" #include "core/loader/EmptyClients.h"
...@@ -1321,6 +1325,49 @@ void Editor::toggleOverwriteModeEnabled() ...@@ -1321,6 +1325,49 @@ void Editor::toggleOverwriteModeEnabled()
frame().selection().setShouldShowBlockCursor(m_overwriteModeEnabled); frame().selection().setShouldShowBlockCursor(m_overwriteModeEnabled);
} }
void Editor::tidyUpHTMLStructure(Document& document)
{
// hasEditableStyle() needs up-to-date ComputedStyle.
document.updateLayoutTreeIfNeeded();
bool needsValidStructure = document.hasEditableStyle() || (document.documentElement() && document.documentElement()->hasEditableStyle());
if (!needsValidStructure)
return;
RefPtrWillBeRawPtr<Element> existingHead = nullptr;
RefPtrWillBeRawPtr<Element> existingBody = nullptr;
Element* currentRoot = document.documentElement();
if (currentRoot) {
if (isHTMLHtmlElement(currentRoot))
return;
if (isHTMLHeadElement(currentRoot))
existingHead = currentRoot;
else if (isHTMLBodyElement(currentRoot))
existingBody = currentRoot;
else if (isHTMLFrameSetElement(currentRoot))
existingBody = currentRoot;
}
// We ensure only "the root is <html>."
// documentElement as rootEditableElement is problematic. So we move
// non-<html> root elements under <body>, and the <body> works as
// rootEditableElement.
document.addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, "document.execCommand() doesn't work with an invalid HTML structure. It is corrected automatically."));
RefPtrWillBeRawPtr<Element> root = HTMLHtmlElement::create(document);
if (existingHead)
root->appendChild(existingHead.release());
RefPtrWillBeRawPtr<Element> body = nullptr;
if (existingBody)
body = existingBody.release();
else
body = HTMLBodyElement::create(document);
if (document.documentElement())
body->appendChild(document.documentElement());
root->appendChild(body.release());
ASSERT(!document.documentElement());
document.appendChild(root.release());
// TODO(tkent): Should we check and move Text node children of <html>?
}
DEFINE_TRACE(Editor) DEFINE_TRACE(Editor)
{ {
visitor->trace(m_frame); visitor->trace(m_frame);
......
third_party/WebKit/Source/core/editing/Editor.h
View file @ 18a1a860
...@@ -229,6 +229,8 @@ public: ...@@ -229,6 +229,8 @@ public:
EditorParagraphSeparator defaultParagraphSeparator() const { return m_defaultParagraphSeparator; } EditorParagraphSeparator defaultParagraphSeparator() const { return m_defaultParagraphSeparator; }
void setDefaultParagraphSeparator(EditorParagraphSeparator separator) { m_defaultParagraphSeparator = separator; } void setDefaultParagraphSeparator(EditorParagraphSeparator separator) { m_defaultParagraphSeparator = separator; }
static void tidyUpHTMLStructure(Document&);
class RevealSelectionScope { class RevealSelectionScope {
WTF_MAKE_NONCOPYABLE(RevealSelectionScope); WTF_MAKE_NONCOPYABLE(RevealSelectionScope);
STACK_ALLOCATED(); STACK_ALLOCATED();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment