2010-09-08 MORITA Hajime <morrita@google.com>
authormorrita@google.com <morrita@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Sep 2010 02:05:26 +0000 (02:05 +0000)
committermorrita@google.com <morrita@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Sep 2010 02:05:26 +0000 (02:05 +0000)
        Reviewed by Tony Chang.

        spelling underline gets lost on backspace
        https://bugs.webkit.org/show_bug.cgi?id=41423

        * editing/spelling/script-tests/TEMPLATE.html: Copied from LayoutTests/editing/selection/script-tests/TEMPLATE.html.
        * editing/spelling/script-tests/spelling-backspace-between-lines.js: Added.
        * editing/spelling/spelling-backspace-between-lines-expected.txt: Added.
        * editing/spelling/spelling-backspace-between-lines.html: Added.
2010-09-08  MORITA Hajime  <morrita@google.com>

        Reviewed by Tony Chang.

        spelling underline gets lost on backspace
        https://bugs.webkit.org/show_bug.cgi?id=41423

        moveParagraphs() did make a DOM range by serializing source range
        and deserializing it back, and markers are gone during the process.
        This change marks that DOM range again.

        Test: editing/spelling/spelling-backspace-between-lines.html

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::moveParagraphs):
        * editing/Editor.cpp:
        (WebCore::Editor::clearMisspellingsAndBadGrammar): Added.
        (WebCore::Editor::markMisspellingsAndBadGrammar): Added.
        * editing/Editor.h:
2010-09-08  MORITA Hajime  <morrita@google.com>

        Reviewed by Tony Chang.

        spelling underline gets lost on backspace
        https://bugs.webkit.org/show_bug.cgi?id=41423

        Switched to use a anchorNode of the selection instead of a focused
        node for finer control of node selection in spellingNode():
        With the focused node, we cannot select other nodes but the first
        child of that node. In some case, we need to inspect these.

        The API is only for LayoutTests, and the change is compatible for
        existing test cases.

        * WebView/WebFrame.mm:
        (spellingNode):

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

LayoutTests/ChangeLog
LayoutTests/editing/spelling/script-tests/TEMPLATE.html [new file with mode: 0644]
LayoutTests/editing/spelling/script-tests/spelling-backspace-between-lines.js [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-backspace-between-lines-expected.txt [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-backspace-between-lines.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/CompositeEditCommand.cpp
WebCore/editing/Editor.cpp
WebCore/editing/Editor.h
WebKit/mac/ChangeLog
WebKit/mac/WebView/WebFrame.mm

index 5b7eb40c162cfea02cd606131c270e3cb19e8462..4c097a481271803c4ed7ccd752406d0b2e320e6e 100644 (file)
@@ -1,3 +1,15 @@
+2010-09-08  MORITA Hajime  <morrita@google.com>
+
+        Reviewed by Tony Chang.
+
+        spelling underline gets lost on backspace
+        https://bugs.webkit.org/show_bug.cgi?id=41423
+
+        * editing/spelling/script-tests/TEMPLATE.html: Copied from LayoutTests/editing/selection/script-tests/TEMPLATE.html.
+        * editing/spelling/script-tests/spelling-backspace-between-lines.js: Added.
+        * editing/spelling/spelling-backspace-between-lines-expected.txt: Added.
+        * editing/spelling/spelling-backspace-between-lines.html: Added.
+
 2010-09-08  Fumitoshi Ukai  <ukai@chromium.org>
 
         Unreviewed.
diff --git a/LayoutTests/editing/spelling/script-tests/TEMPLATE.html b/LayoutTests/editing/spelling/script-tests/TEMPLATE.html
new file mode 100644 (file)
index 0000000..fe6e50f
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/js-test-selection-shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="YOUR_JS_FILE_HERE"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/script-tests/spelling-backspace-between-lines.js b/LayoutTests/editing/spelling/script-tests/spelling-backspace-between-lines.js
new file mode 100644 (file)
index 0000000..d3663d2
--- /dev/null
@@ -0,0 +1,65 @@
+
+description('For Bug 41423: Spelling marker should remain after hitting a backspace key.');
+
+var testRoot = document.createElement("div");
+document.body.insertBefore(testRoot, document.body.firstChild);
+
+function setup(targetName)
+{
+    testRoot.innerHTML = "<div id='" + targetName + "' contentEditable><div>OK</div><div>OK zz OK</div></div>";
+    document.getElementById(targetName).focus();
+    return document.getSelection();
+}
+
+function firstLineText()
+{
+    return testRoot.firstChild.firstChild.innerText.trim();
+}
+
+function testWithDelete()
+{
+    window.sel = setup("target1");
+
+    sel.modify("move", "forward", "line");
+    for (var i = 0; i < 3; i++) // 3 for ["OK, "zz", "OK"].length
+        sel.modify("move", "forward", "word");
+
+    shouldBe("firstLineText()", "'OK'");
+    shouldBe("sel.anchorNode.data", "'OK zz OK'");
+    shouldBe("textInputController.hasSpellingMarker(3, 2)", "1");
+
+    sel.modify("move", "left", "lineboundary");
+    document.execCommand("Delete", false);
+    sel.modify("move", "right", "line"); // Moves to the line ending to focus the "OK zz OK" text.
+
+    shouldBe("sel.anchorNode.data", "'OK zz OK'");
+    shouldBe("firstLineText()", "'OKOK zz OK'");
+    shouldBe("textInputController.hasSpellingMarker(3, 2)", "1");
+}
+
+function testWithForwardDelete()
+{
+    window.sel = setup("target1");
+
+    sel.modify("move", "forward", "line");
+    for (var i = 0; i < 3; i++) // 3 for ["OK, "zz", "OK"].length
+        sel.modify("move", "forward", "word");
+
+    shouldBe("firstLineText()", "'OK'");
+    shouldBe("sel.anchorNode.data", "'OK zz OK'");
+    shouldBe("textInputController.hasSpellingMarker(3, 2)", "1");
+
+    sel.modify("move", "left", "line");
+    document.execCommand("ForwardDelete", false);
+    sel.modify("move", "right", "line"); // Moves to the line ending to focus the "OK zz OK" text.
+
+    shouldBe("firstLineText()", "'OKOK zz OK'");
+    shouldBe("sel.anchorNode.data", "'OK zz OK'");
+    shouldBe("textInputController.hasSpellingMarker(3, 2)", "1");
+}
+
+testWithDelete();
+testWithForwardDelete();
+testRoot.style.display = "none";
+
+var successfullyParsed = true;
diff --git a/LayoutTests/editing/spelling/spelling-backspace-between-lines-expected.txt b/LayoutTests/editing/spelling/spelling-backspace-between-lines-expected.txt
new file mode 100644 (file)
index 0000000..03841f3
--- /dev/null
@@ -0,0 +1,21 @@
+For Bug 41423: Spelling marker should remain after hitting a backspace key.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS firstLineText() is 'OK'
+PASS sel.anchorNode.data is 'OK zz OK'
+PASS textInputController.hasSpellingMarker(3, 2) is 1
+PASS sel.anchorNode.data is 'OK zz OK'
+PASS firstLineText() is 'OKOK zz OK'
+PASS textInputController.hasSpellingMarker(3, 2) is 1
+PASS firstLineText() is 'OK'
+PASS sel.anchorNode.data is 'OK zz OK'
+PASS textInputController.hasSpellingMarker(3, 2) is 1
+PASS firstLineText() is 'OKOK zz OK'
+PASS sel.anchorNode.data is 'OK zz OK'
+PASS textInputController.hasSpellingMarker(3, 2) is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/spelling/spelling-backspace-between-lines.html b/LayoutTests/editing/spelling/spelling-backspace-between-lines.html
new file mode 100644 (file)
index 0000000..bd20e96
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/js-test-selection-shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/spelling-backspace-between-lines.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index d6e657e68c955e92efce32a431cc5eaab8cbe306..76aea1f6ac7636b6c7cc483ebcddd3109d4e2e0a 100644 (file)
@@ -1,3 +1,23 @@
+2010-09-08  MORITA Hajime  <morrita@google.com>
+
+        Reviewed by Tony Chang.
+
+        spelling underline gets lost on backspace
+        https://bugs.webkit.org/show_bug.cgi?id=41423
+
+        moveParagraphs() did make a DOM range by serializing source range
+        and deserializing it back, and markers are gone during the process.
+        This change marks that DOM range again.
+
+        Test: editing/spelling/spelling-backspace-between-lines.html
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraphs):
+        * editing/Editor.cpp:
+        (WebCore::Editor::clearMisspellingsAndBadGrammar): Added.
+        (WebCore::Editor::markMisspellingsAndBadGrammar): Added.
+        * editing/Editor.h:
+
 2010-09-08  Adam Barth  <abarth@webkit.org>
 
         Reviewed by Eric Seidel.
index aa37193d4e29250ce16ec86a03c9eb60309f71d2..356a7170621a12868ed02d395ff5dd149fd6ef96 100644 (file)
@@ -36,6 +36,7 @@
 #include "Document.h"
 #include "DocumentFragment.h"
 #include "EditorInsertAction.h"
+#include "Frame.h"
 #include "HTMLElement.h"
 #include "HTMLNames.h"
 #include "InlineTextBox.h"
@@ -939,6 +940,7 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
     // FIXME (5098931): We should add a new insert action "WebViewInsertActionMoved" and call shouldInsertFragment here.
     
     setEndingSelection(VisibleSelection(start, end, DOWNSTREAM));
+    document()->frame()->editor()->clearMisspellingsAndBadGrammar(endingSelection());
     deleteSelection(false, false, false, false);
 
     ASSERT(destination.deepEquivalent().node()->inDocument());
@@ -968,7 +970,9 @@ void CompositeEditCommand::moveParagraphs(const VisiblePosition& startOfParagrap
     setEndingSelection(destination);
     ASSERT(endingSelection().isCaretOrRange());
     applyCommandToComposite(ReplaceSelectionCommand::create(document(), fragment, true, false, !preserveStyle, false, true));
-    
+
+    document()->frame()->editor()->markMisspellingsAndBadGrammar(endingSelection());
+
     // If the selection is in an empty paragraph, restore styles from the old empty paragraph to the new empty paragraph.
     bool selectionIsEmptyParagraph = endingSelection().isCaret() && isStartOfParagraph(endingSelection().visibleStart()) && isEndOfParagraph(endingSelection().visibleStart());
     if (styleInEmptyParagraph && selectionIsEmptyParagraph)
index 9e41a62487c2173d525edd954c57819e537aa07d..d3b8376dbb5ca4ac7d5fdc7a510ca6cc61b44ba9 100644 (file)
@@ -2353,6 +2353,29 @@ bool Editor::spellingPanelIsShowing()
     return client()->spellingUIIsShowing();
 }
 
+void Editor::clearMisspellingsAndBadGrammar(const VisibleSelection &movingSelection)
+{
+    RefPtr<Range> selectedRange = movingSelection.toNormalizedRange();
+    if (selectedRange) {
+        frame()->document()->markers()->removeMarkers(selectedRange.get(), DocumentMarker::Spelling);
+        frame()->document()->markers()->removeMarkers(selectedRange.get(), DocumentMarker::Grammar);
+    }
+}
+
+void Editor::markMisspellingsAndBadGrammar(const VisibleSelection &movingSelection)
+{
+    bool markSpelling = isContinuousSpellCheckingEnabled();
+    bool markGrammar = markSpelling && isGrammarCheckingEnabled();
+
+    if (markSpelling) {
+        RefPtr<Range> unusedFirstMisspellingRange;
+        markMisspellings(movingSelection, unusedFirstMisspellingRange);
+    }
+
+    if (markGrammar)
+        markBadGrammar(movingSelection);
+}
+
 void Editor::markMisspellingsAfterTypingToPosition(const VisiblePosition &p)
 {
 #if PLATFORM(MAC) && !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD)
index 78e89b485ea982736f275853edaf06f1420ce222..79552b456b9d034c4dc4bbedc344510978a426e7 100644 (file)
@@ -314,6 +314,9 @@ public:
 
     // This is only called on the mac where paste is implemented primarily at the WebKit level.
     void pasteAsPlainTextBypassingDHTML();
+    void clearMisspellingsAndBadGrammar(const VisibleSelection&);
+    void markMisspellingsAndBadGrammar(const VisibleSelection&);
 
     Node* findEventTargetFrom(const VisibleSelection& selection) const;
 private:
index 09bfd690e3c3c2d448a9efeb6dea237fa7ee211c..80a6b31105c51916496b9184e1941d7dbe77d16d 100644 (file)
@@ -1,3 +1,21 @@
+2010-09-08  MORITA Hajime  <morrita@google.com>
+
+        Reviewed by Tony Chang.
+
+        spelling underline gets lost on backspace
+        https://bugs.webkit.org/show_bug.cgi?id=41423
+
+        Switched to use a anchorNode of the selection instead of a focused
+        node for finer control of node selection in spellingNode():
+        With the focused node, we cannot select other nodes but the first
+        child of that node. In some case, we need to inspect these.
+
+        The API is only for LayoutTests, and the change is compatible for
+        existing test cases.
+        
+        * WebView/WebFrame.mm:
+        (spellingNode):
+
 2010-09-08  Adam Barth  <abarth@webkit.org>
 
         Rubber-stamped by Eric Seidel.
index a3b6f9e755441356e89449d611fbfece63d2b34b..5b288333358977321dad7064bfb44ef7d92718aa 100644 (file)
@@ -1368,7 +1368,7 @@ static inline WebDataSource *dataSource(DocumentLoader* loader)
 
 static Node* spellingNode(Frame* coreFrame)
 {
-    Node* focusedNode = coreFrame->document()->focusedNode();
+    Node* focusedNode = coreFrame->selection()->start().node();
     if (!focusedNode || !focusedNode->renderer())
         return 0;