2011-03-21 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 3 Apr 2011 07:54:33 +0000 (07:54 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 3 Apr 2011 07:54:33 +0000 (07:54 +0000)
        Reviewed by Eric Seidel.

        editing commands shouldn't run when there's no body
        https://bugs.webkit.org/show_bug.cgi?id=56771

        The bug was caused by WebKit's not checking the existence of root editable element
        in enabled* functions. Although isContentEditable returns true whenever we're in design mode,
        we should not run editing commands in a document without a body element editable because
        doing so results in appending a non-body element to the document node.

        Fixed the bug by modifying various enabled* functions to ensure we have a root editable element.
        New behavior tries to match that of Firefox except StyleWithCSS, which Firefox seems to ignore
        when there are no body element. Since StyleWithCSS is a document's state or property, we allow
        execCommand('StyleWithCSS') even in a document without a body element.

        WebKit's and Firefox's behaviors also deviate in insert-image-with-selecting-document.html.
        Whereas WebKit respects selection set by script and ignores execCommand, Firefox modifies
        the selection when document.write("x") is ran and successfully inserts image.

        Thus, empty-document-delete.html and empty-document-justify-right.html both pass on Firefox
        while empty-document-stylewithcss.html and insert-image-with-selecting-document.html both fail.

        Since Internet Explorer does not allow execCommand to run under design mode properly, we could
        not test its behavior.

        Tests: editing/editability/empty-document-delete.html
               editing/editability/empty-document-justify-right.html
               editing/editability/empty-document-stylewithcss.html
               editing/execCommand/insert-image-with-selecting-document.html

        * editing/Editor.cpp:
        (WebCore::Editor::canEdit): Verify that the root editable element exists
        instead of just checking that selection endpoints are editable because
        selection endpoints could be document node without a body element in design mode
        and we don't want to consider such a document editable.
        (WebCore::Editor::canDelete): Ditto.
        * editing/EditorCommand.cpp:
        (WebCore::enabledInEditableText): Ditto.
        (WebCore::enabledInRichlyEditableText): Ditto.
        (WebCore::enabledDelete): Call enabledCut and enabledInEditableText instead
        of duplicating the code in order to fix the same bug.
2011-03-21  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Eric Seidel.

        editing commands shouldn't run when there's no body
        https://bugs.webkit.org/show_bug.cgi?id=56771

        Added tests to ensure WebKit does not crash when attempted to execute editing commands
        in an empty document. Also added a test to ensure WebKit does not crash when InsertImage
        is executed with selection endpoints being document. WebKit should ignore such attempts
        and should not crash.

        * editing/editability/empty-document-delete-expected.txt: Added.
        * editing/editability/empty-document-delete.html: Added.
        * editing/editability/empty-document-justify-right-expected.txt: Added.
        * editing/editability/empty-document-justify-right.html: Added.
        * editing/editability/empty-document-stylewithcss-expected.txt: Added.
        * editing/editability/empty-document-stylewithcss.html: Added.
        * editing/execCommand/insert-image-with-selecting-document-expected.txt: Added.
        * editing/execCommand/insert-image-with-selecting-document.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@82791 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/editability/empty-document-delete-expected.txt [new file with mode: 0644]
LayoutTests/editing/editability/empty-document-delete.html [new file with mode: 0644]
LayoutTests/editing/editability/empty-document-justify-right-expected.txt [new file with mode: 0644]
LayoutTests/editing/editability/empty-document-justify-right.html [new file with mode: 0644]
LayoutTests/editing/editability/empty-document-stylewithcss-expected.txt [new file with mode: 0644]
LayoutTests/editing/editability/empty-document-stylewithcss.html [new file with mode: 0644]
LayoutTests/editing/execCommand/insert-image-with-selecting-document-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/insert-image-with-selecting-document.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/EditorCommand.cpp

index 0501aea..5194302 100644 (file)
@@ -1,3 +1,24 @@
+2011-03-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        editing commands shouldn't run when there's no body
+        https://bugs.webkit.org/show_bug.cgi?id=56771
+
+        Added tests to ensure WebKit does not crash when attempted to execute editing commands
+        in an empty document. Also added a test to ensure WebKit does not crash when InsertImage
+        is executed with selection endpoints being document. WebKit should ignore such attempts
+        and should not crash.
+
+        * editing/editability/empty-document-delete-expected.txt: Added.
+        * editing/editability/empty-document-delete.html: Added.
+        * editing/editability/empty-document-justify-right-expected.txt: Added.
+        * editing/editability/empty-document-justify-right.html: Added.
+        * editing/editability/empty-document-stylewithcss-expected.txt: Added.
+        * editing/editability/empty-document-stylewithcss.html: Added.
+        * editing/execCommand/insert-image-with-selecting-document-expected.txt: Added.
+        * editing/execCommand/insert-image-with-selecting-document.html: Added.
+
 2011-04-02  Dan Bernstein  <mitz@apple.com>
 
         Updated results showing a progression after r82787. The current time display
diff --git a/LayoutTests/editing/editability/empty-document-delete-expected.txt b/LayoutTests/editing/editability/empty-document-delete-expected.txt
new file mode 100644 (file)
index 0000000..e530cf6
--- /dev/null
@@ -0,0 +1,3 @@
+This test ensures WebKit does not crash when executing Delete in an empty document.
+
+PASS
diff --git a/LayoutTests/editing/editability/empty-document-delete.html b/LayoutTests/editing/editability/empty-document-delete.html
new file mode 100644 (file)
index 0000000..51c8c6f
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function runTest() {
+    document.designMode="on";
+    document.open();
+    window.getSelection().addRange(document.createRange());
+    document.execCommand("Delete", false, null);
+    document.writeln('This test ensures WebKit does not crash when executing Delete in an empty document.<br><br>PASS');
+}
+</script>
+</head>
+<body onload="runTest()"></body>
+</html>
diff --git a/LayoutTests/editing/editability/empty-document-justify-right-expected.txt b/LayoutTests/editing/editability/empty-document-justify-right-expected.txt
new file mode 100644 (file)
index 0000000..9e8f240
--- /dev/null
@@ -0,0 +1,3 @@
+This test ensures WebKit does not crash when executing JustifyRight in an empty document.
+
+PASS
diff --git a/LayoutTests/editing/editability/empty-document-justify-right.html b/LayoutTests/editing/editability/empty-document-justify-right.html
new file mode 100644 (file)
index 0000000..7a08719
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function runTest() {
+    document.designMode="on";
+    document.open();
+    window.getSelection().addRange(document.createRange());
+    document.execCommand("JustifyRight", false, null);
+    document.writeln('This test ensures WebKit does not crash when executing JustifyRight in an empty document.<br><br>PASS');
+}
+</script>
+</head>
+<body onload="runTest()"></body>
+</html>
diff --git a/LayoutTests/editing/editability/empty-document-stylewithcss-expected.txt b/LayoutTests/editing/editability/empty-document-stylewithcss-expected.txt
new file mode 100644 (file)
index 0000000..797da42
--- /dev/null
@@ -0,0 +1,4 @@
+This test ensures WebKit executes StyleWithCSS properly even in an empty document.
+First negation:PASS
+Second negation:PASS
+
diff --git a/LayoutTests/editing/editability/empty-document-stylewithcss.html b/LayoutTests/editing/editability/empty-document-stylewithcss.html
new file mode 100644 (file)
index 0000000..90e74b0
--- /dev/null
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function runTest() {
+    document.designMode="on";
+    document.open();
+    window.getSelection().addRange(document.createRange());
+
+    var initialValue = document.queryCommandValue('StyleWithCSS');
+    document.execCommand("StyleWithCSS", false, !eval(initialValue));
+    document.writeln('hello');
+
+    document.open();
+    window.getSelection().addRange(document.createRange());
+    var valueAfterFirstNegation = document.queryCommandValue('StyleWithCSS');
+
+    document.execCommand("StyleWithCSS", false, !eval(valueAfterFirstNegation));
+    document.writeln('world');
+    var valueAfterSecondNegation = document.queryCommandValue('StyleWithCSS');
+
+    document.open();
+    document.writeln('This test ensures WebKit executes StyleWithCSS properly even in an empty document.<br>');
+    document.write('First negation:' + (initialValue !== valueAfterFirstNegation ? 'PASS' : 'FAIL') + '<br>');
+    document.write('Second negation:' + (valueAfterFirstNegation !== valueAfterSecondNegation ? 'PASS' : 'FAIL') + '<br>');
+}
+</script>
+</head>
+<body onload="runTest()"></body>
+</html>
diff --git a/LayoutTests/editing/execCommand/insert-image-with-selecting-document-expected.txt b/LayoutTests/editing/execCommand/insert-image-with-selecting-document-expected.txt
new file mode 100644 (file)
index 0000000..8d7dcc4
--- /dev/null
@@ -0,0 +1,3 @@
+This test ensures WebKit does not crash when executing InsertImage with selection endpoints are the document node.
+
+PASS
diff --git a/LayoutTests/editing/execCommand/insert-image-with-selecting-document.html b/LayoutTests/editing/execCommand/insert-image-with-selecting-document.html
new file mode 100644 (file)
index 0000000..9db56d2
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function runTest() {
+    document.designMode="on";
+    document.open();
+    window.getSelection().setPosition(document, 0);
+    document.write("x");
+    document.execCommand("InsertImage");
+    document.open();
+    document.writeln('This test ensures WebKit does not crash when executing InsertImage with selection endpoints are the document node.<br><br>PASS');
+}
+</script>
+</head>
+<body onload="runTest()"></body>
+</html>
index 139671a..a835316 100644 (file)
@@ -1,3 +1,47 @@
+2011-03-21  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        editing commands shouldn't run when there's no body
+        https://bugs.webkit.org/show_bug.cgi?id=56771
+
+        The bug was caused by WebKit's not checking the existence of root editable element
+        in enabled* functions. Although isContentEditable returns true whenever we're in design mode,
+        we should not run editing commands in a document without a body element editable because
+        doing so results in appending a non-body element to the document node.
+
+        Fixed the bug by modifying various enabled* functions to ensure we have a root editable element.
+        New behavior tries to match that of Firefox except StyleWithCSS, which Firefox seems to ignore
+        when there are no body element. Since StyleWithCSS is a document's state or property, we allow
+        execCommand('StyleWithCSS') even in a document without a body element.
+
+        WebKit's and Firefox's behaviors also deviate in insert-image-with-selecting-document.html.
+        Whereas WebKit respects selection set by script and ignores execCommand, Firefox modifies
+        the selection when document.write("x") is ran and successfully inserts image.
+
+        Thus, empty-document-delete.html and empty-document-justify-right.html both pass on Firefox
+        while empty-document-stylewithcss.html and insert-image-with-selecting-document.html both fail.
+
+        Since Internet Explorer does not allow execCommand to run under design mode properly, we could
+        not test its behavior.
+
+        Tests: editing/editability/empty-document-delete.html
+               editing/editability/empty-document-justify-right.html
+               editing/editability/empty-document-stylewithcss.html
+               editing/execCommand/insert-image-with-selecting-document.html
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::canEdit): Verify that the root editable element exists
+        instead of just checking that selection endpoints are editable because
+        selection endpoints could be document node without a body element in design mode
+        and we don't want to consider such a document editable.
+        (WebCore::Editor::canDelete): Ditto.
+        * editing/EditorCommand.cpp:
+        (WebCore::enabledInEditableText): Ditto.
+        (WebCore::enabledInRichlyEditableText): Ditto.
+        (WebCore::enabledDelete): Call enabledCut and enabledInEditableText instead
+        of duplicating the code in order to fix the same bug.
+
 2011-04-02  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Maciej Stachowiak.
index 2607c11..05fa1d8 100644 (file)
@@ -210,7 +210,7 @@ bool Editor::handleTextEvent(TextEvent* event)
 
 bool Editor::canEdit() const
 {
-    return m_frame->selection()->isContentEditable();
+    return m_frame->selection()->rootEditableElement();
 }
 
 bool Editor::canEditRichly() const
@@ -278,7 +278,7 @@ bool Editor::canPaste() const
 bool Editor::canDelete() const
 {
     SelectionController* selection = m_frame->selection();
-    return selection->isRange() && selection->isContentEditable();
+    return selection->isRange() && selection->rootEditableElement();
 }
 
 bool Editor::canDeleteRange(Range* range) const
index 8ea37bb..75a20d7 100644 (file)
@@ -1196,27 +1196,27 @@ static bool enabledCut(Frame* frame, Event*, EditorCommandSource)
     return frame->editor()->canDHTMLCut() || frame->editor()->canCut();
 }
 
+static bool enabledInEditableText(Frame* frame, Event* event, EditorCommandSource)
+{
+    return frame->editor()->selectionForCommand(event).rootEditableElement();
+}
+
 static bool enabledDelete(Frame* frame, Event* event, EditorCommandSource source)
 {
     switch (source) {
     case CommandFromMenuOrKeyBinding:
         // "Delete" from menu only affects selected range, just like Cut but without affecting pasteboard
-        return frame->editor()->canDHTMLCut() || frame->editor()->canCut();
+        return enabledCut(frame, event, source);
     case CommandFromDOM:
     case CommandFromDOMWithUserInterface:
         // "Delete" from DOM is like delete/backspace keypress, affects selected range if non-empty,
         // otherwise removes a character
-        return frame->editor()->selectionForCommand(event).isContentEditable();
+        return enabledInEditableText(frame, event, source);
     }
     ASSERT_NOT_REACHED();
     return false;
 }
 
-static bool enabledInEditableText(Frame* frame, Event* event, EditorCommandSource)
-{
-    return frame->editor()->selectionForCommand(event).isContentEditable();
-}
-
 static bool enabledInEditableTextOrCaretBrowsing(Frame* frame, Event* event, EditorCommandSource)
 {
     // The EditorCommandSource parameter is unused in enabledInEditableText, so just pass a dummy variable
@@ -1225,7 +1225,7 @@ static bool enabledInEditableTextOrCaretBrowsing(Frame* frame, Event* event, Edi
 
 static bool enabledInRichlyEditableText(Frame* frame, Event*, EditorCommandSource)
 {
-    return frame->selection()->isCaretOrRange() && frame->selection()->isContentRichlyEditable();
+    return frame->selection()->isCaretOrRange() && frame->selection()->isContentRichlyEditable() && frame->selection()->rootEditableElement();
 }
 
 static bool enabledPaste(Frame* frame, Event*, EditorCommandSource)