Title: [120423] trunk
Revision
120423
Author
hb...@chromium.org
Date
2012-06-15 01:41:30 -0700 (Fri, 15 Jun 2012)

Log Message

Allow platforms to choose whether to remove markers on editing
https://bugs.webkit.org/show_bug.cgi?id=88838

Reviewed by Hajime Morita.

Source/WebCore:

This change allows platforms to choose whether to remove markers on a word being
edited. WebKit does not remove markers when we move a selection to a markered
word on platforms that shouldEraseMarkersAfterChangeSelection returns false.
On such platforms, WebKit expects to set WTF_USE_MARKER_REMOVAL_UPON_EDITING to
1 so Editor::updateMarkersForWordsAffectedByEditing can remove markers. This
change also checks the return value of shouldEraseMarkersAfterChangeSelection so
platform can choose it. This change also adds grammar markers so it can also
remove grammar markers.

Test: editing/spelling/grammar-edit-word.html

* editing/Editor.cpp:
(WebCore::Editor::updateMarkersForWordsAffectedByEditing):

Source/WebKit/chromium:

This change implements EditorClientImpl::checkGrammarOfString so
DumpRenderTree can run grammar tests.

* src/EditorClientImpl.cpp:
(WebKit::EditorClientImpl::isGrammarCheckingEnabled): Return true also when unified text-checking is enabled.
(WebKit::EditorClientImpl::shouldEraseMarkersAfterChangeSelection): ditto.
(WebKit::EditorClientImpl::checkGrammarOfString): Implement this function with our unified text-checker.

Tools:

This change implements WebViewHost::checkTextOfParagraph so DumpRenderTree can
run grammar tests.

* DumpRenderTree/chromium/WebViewHost.cpp:
(WebViewHost::checkTextOfParagraph): Implement this function with our mock spell checker and grammar checker.
* DumpRenderTree/chromium/WebViewHost.h:
(WebViewHost): Override WebSpellCheckClient::checkTextOfParagraph.

LayoutTests:

This change adds a test that verifies WebKit removes a grammar marker from a
grammatically-incorrect word when editing the word.

* editing/spelling/grammar-edit-word-expected.txt: Added.
* editing/spelling/grammar-edit-word.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120422 => 120423)


--- trunk/LayoutTests/ChangeLog	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/LayoutTests/ChangeLog	2012-06-15 08:41:30 UTC (rev 120423)
@@ -1,3 +1,16 @@
+2012-06-15  Hironori Bono  <hb...@chromium.org>
+
+        Allow platforms to choose whether to remove markers on editing
+        https://bugs.webkit.org/show_bug.cgi?id=88838
+
+        Reviewed by Hajime Morita.
+
+        This change adds a test that verifies WebKit removes a grammar marker from a
+        grammatically-incorrect word when editing the word.
+
+        * editing/spelling/grammar-edit-word-expected.txt: Added.
+        * editing/spelling/grammar-edit-word.html: Added.
+
 2012-06-15  Zan Dobersek  <zandober...@gmail.com>
 
         Unreviewed GTK gardening, adding test expectation for

Added: trunk/LayoutTests/editing/spelling/grammar-edit-word-expected.txt (0 => 120423)


--- trunk/LayoutTests/editing/spelling/grammar-edit-word-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/spelling/grammar-edit-word-expected.txt	2012-06-15 08:41:30 UTC (rev 120423)
@@ -0,0 +1,27 @@
+Test if WebKit removes grammar markers when we edit a grammatically-incorrect word. To test manually, type a grammatically-incorrect sentence "You has the right." and type a backspace key to delete the last character of "has". This test succeeds when "ha" does not have grammar markers.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+document.execCommand("InsertText", false, "You has the right.")
+PASS internals.hasGrammarMarker(document, 4, 3) is true
+Delete the end of this sentence until it becomes "You ha".
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+layoutTestController.execCommand("DeleteBackward")
+PASS internals.hasGrammarMarker(document, 4, 2) is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+You ha
+
+

Added: trunk/LayoutTests/editing/spelling/grammar-edit-word.html (0 => 120423)


--- trunk/LayoutTests/editing/spelling/grammar-edit-word.html	                        (rev 0)
+++ trunk/LayoutTests/editing/spelling/grammar-edit-word.html	2012-06-15 08:41:30 UTC (rev 120423)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<title>Editing a grammatically-incorrect word</title> 
+</head> 
+<body>
+<div id="src" contenteditable="true" spellcheck="true"></div><br/>
+<script language="_javascript_">
+description('Test if WebKit removes grammar markers when we edit a grammatically-incorrect word. To test manually, type a grammatically-incorrect sentence "You has the right." and type a backspace key to delete the last character of "has". This test succeeds when "ha" does not have grammar markers.');
+
+internals.settings.setUnifiedTextCheckingEnabled(true);
+var target = document.getElementById('src');
+target.focus();
+
+evalAndLog('document.execCommand("InsertText", false, "You has the right.")');
+shouldBeTrue('internals.hasGrammarMarker(document, 4, 3)');
+
+debug('Delete the end of this sentence until it becomes "You ha".');
+for (var i = 0; i < 12; ++i)
+    evalAndLog('layoutTestController.execCommand("DeleteBackward")');
+shouldBeFalse('internals.hasGrammarMarker(document, 4, 2)');
+
+internals.settings.setUnifiedTextCheckingEnabled(false);
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (120422 => 120423)


--- trunk/Source/WebCore/ChangeLog	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/Source/WebCore/ChangeLog	2012-06-15 08:41:30 UTC (rev 120423)
@@ -1,3 +1,24 @@
+2012-06-15  Hironori Bono  <hb...@chromium.org>
+
+        Allow platforms to choose whether to remove markers on editing
+        https://bugs.webkit.org/show_bug.cgi?id=88838
+
+        Reviewed by Hajime Morita.
+
+        This change allows platforms to choose whether to remove markers on a word being
+        edited. WebKit does not remove markers when we move a selection to a markered
+        word on platforms that shouldEraseMarkersAfterChangeSelection returns false.
+        On such platforms, WebKit expects to set WTF_USE_MARKER_REMOVAL_UPON_EDITING to
+        1 so Editor::updateMarkersForWordsAffectedByEditing can remove markers. This
+        change also checks the return value of shouldEraseMarkersAfterChangeSelection so
+        platform can choose it. This change also adds grammar markers so it can also
+        remove grammar markers.
+
+        Test: editing/spelling/grammar-edit-word.html
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::updateMarkersForWordsAffectedByEditing):
+
 2012-06-15  Andrey Adaikin  <aand...@chromium.org>
 
         Web Inspector: [WebGL] Simple implementation of the InjectedWebGLScriptSource to support capturing WebGL calls for a frame

Modified: trunk/Source/WebCore/editing/Editor.cpp (120422 => 120423)


--- trunk/Source/WebCore/editing/Editor.cpp	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/Source/WebCore/editing/Editor.cpp	2012-06-15 08:41:30 UTC (rev 120423)
@@ -2203,7 +2203,7 @@
 
 void Editor::updateMarkersForWordsAffectedByEditing(bool doNotRemoveIfSelectionAtWordBoundary)
 {
-    if (!m_alternativeTextController->shouldRemoveMarkersUponEditing())
+    if (!m_alternativeTextController->shouldRemoveMarkersUponEditing() && (!textChecker() || textChecker()->shouldEraseMarkersAfterChangeSelection(TextCheckingTypeSpelling)))
         return;
 
     // We want to remove the markers from a word if an editing command will change the word. This can happen in one of
@@ -2271,7 +2271,7 @@
     for (size_t i = 0; i < markers.size(); ++i)
         m_alternativeTextController->removeDictationAlternativesForMarker(markers[i]);
 
-    document->markers()->removeMarkers(wordRange.get(), DocumentMarker::Spelling | DocumentMarker::CorrectionIndicator | DocumentMarker::SpellCheckingExemption | DocumentMarker::DictationAlternatives, DocumentMarkerController::RemovePartiallyOverlappingMarker);
+    document->markers()->removeMarkers(wordRange.get(), DocumentMarker::Spelling | DocumentMarker::Grammar | DocumentMarker::CorrectionIndicator | DocumentMarker::SpellCheckingExemption | DocumentMarker::DictationAlternatives, DocumentMarkerController::RemovePartiallyOverlappingMarker);
     document->markers()->clearDescriptionOnMarkersIntersectingRange(wordRange.get(), DocumentMarker::Replacement);
 }
 

Modified: trunk/Source/WebKit/chromium/ChangeLog (120422 => 120423)


--- trunk/Source/WebKit/chromium/ChangeLog	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/Source/WebKit/chromium/ChangeLog	2012-06-15 08:41:30 UTC (rev 120423)
@@ -1,3 +1,18 @@
+2012-06-15  Hironori Bono  <hb...@chromium.org>
+
+        Allow platforms to choose whether to remove markers on editing
+        https://bugs.webkit.org/show_bug.cgi?id=88838
+
+        Reviewed by Hajime Morita.
+
+        This change implements EditorClientImpl::checkGrammarOfString so
+        DumpRenderTree can run grammar tests.
+
+        * src/EditorClientImpl.cpp:
+        (WebKit::EditorClientImpl::isGrammarCheckingEnabled): Return true also when unified text-checking is enabled.
+        (WebKit::EditorClientImpl::shouldEraseMarkersAfterChangeSelection): ditto.
+        (WebKit::EditorClientImpl::checkGrammarOfString): Implement this function with our unified text-checker.
+
 2012-06-14  Kent Tamura  <tk...@chromium.org>
 
         Support file extensions in HTMLInputElement::accept

Modified: trunk/Source/WebKit/chromium/src/EditorClientImpl.cpp (120422 => 120423)


--- trunk/Source/WebKit/chromium/src/EditorClientImpl.cpp	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/Source/WebKit/chromium/src/EditorClientImpl.cpp	2012-06-15 08:41:30 UTC (rev 120423)
@@ -170,7 +170,7 @@
 bool EditorClientImpl::isGrammarCheckingEnabled()
 {
     const Frame* frame = m_webView->focusedWebCoreFrame();
-    return frame && frame->settings() && frame->settings()->asynchronousSpellCheckingEnabled();
+    return frame && frame->settings() && (frame->settings()->asynchronousSpellCheckingEnabled() || frame->settings()->unifiedTextCheckerEnabled());
 }
 
 void EditorClientImpl::toggleGrammarChecking()
@@ -703,7 +703,7 @@
 bool EditorClientImpl::shouldEraseMarkersAfterChangeSelection(TextCheckingType type) const
 {
     const Frame* frame = m_webView->focusedWebCoreFrame();
-    return !frame || !frame->settings() || !frame->settings()->asynchronousSpellCheckingEnabled();
+    return !frame || !frame->settings() || (!frame->settings()->asynchronousSpellCheckingEnabled() && !frame->settings()->unifiedTextCheckerEnabled());
 }
 
 void EditorClientImpl::ignoreWordInSpellDocument(const String&)
@@ -766,16 +766,39 @@
     return String();
 }
 
-void EditorClientImpl::checkGrammarOfString(const UChar*, int length,
-                                            WTF::Vector<GrammarDetail>&,
-                                            int* badGrammarLocation,
-                                            int* badGrammarLength)
+void EditorClientImpl::checkGrammarOfString(const UChar* text, int length, WTF::Vector<GrammarDetail>& details, int* badGrammarLocation, int* badGrammarLength)
 {
-    notImplemented();
     if (badGrammarLocation)
         *badGrammarLocation = -1;
     if (badGrammarLength)
         *badGrammarLength = 0;
+
+    if (!m_webView->spellCheckClient())
+        return;
+    WebVector<WebTextCheckingResult> webResults;
+    m_webView->spellCheckClient()->checkTextOfParagraph(WebString(text, length), WebTextCheckingTypeGrammar, &webResults);
+    if (!webResults.size())
+        return;
+
+    // Convert a list of WebTextCheckingResults to a list of GrammarDetails. If
+    // the converted vector of GrammarDetails has grammar errors, we set
+    // badGrammarLocation and badGrammarLength to tell WebKit that the input
+    // text has grammar errors.
+    for (size_t i = 0; i < webResults.size(); ++i) {
+        if (webResults[i].type == WebTextCheckingTypeGrammar) {
+            GrammarDetail detail;
+            detail.location = webResults[i].location;
+            detail.length = webResults[i].length;
+            detail.userDescription = webResults[i].replacement;
+            details.append(detail);
+        }
+    }
+    if (!details.size())
+        return;
+    if (badGrammarLocation)
+        *badGrammarLocation = 0;
+    if (badGrammarLength)
+        *badGrammarLength = length;
 }
 
 void EditorClientImpl::checkTextOfParagraph(const UChar* text, int length,

Modified: trunk/Tools/ChangeLog (120422 => 120423)


--- trunk/Tools/ChangeLog	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/Tools/ChangeLog	2012-06-15 08:41:30 UTC (rev 120423)
@@ -1,3 +1,18 @@
+2012-06-15  Hironori Bono  <hb...@chromium.org>
+
+        Allow platforms to choose whether to remove markers on editing
+        https://bugs.webkit.org/show_bug.cgi?id=88838
+
+        Reviewed by Hajime Morita.
+
+        This change implements WebViewHost::checkTextOfParagraph so DumpRenderTree can
+        run grammar tests.
+
+        * DumpRenderTree/chromium/WebViewHost.cpp:
+        (WebViewHost::checkTextOfParagraph): Implement this function with our mock spell checker and grammar checker.
+        * DumpRenderTree/chromium/WebViewHost.h:
+        (WebViewHost): Override WebSpellCheckClient::checkTextOfParagraph.
+
 2012-06-15  Sheriff Bot  <webkit.review....@gmail.com>
 
         Unreviewed, rolling out r120370.

Modified: trunk/Tools/DumpRenderTree/chromium/WebViewHost.cpp (120422 => 120423)


--- trunk/Tools/DumpRenderTree/chromium/WebViewHost.cpp	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/Tools/DumpRenderTree/chromium/WebViewHost.cpp	2012-06-15 08:41:30 UTC (rev 120423)
@@ -467,6 +467,32 @@
     m_spellcheck.spellCheckWord(text, &misspelledOffset, &misspelledLength);
 }
 
+void WebViewHost::checkTextOfParagraph(const WebString& text, WebTextCheckingTypeMask mask, WebVector<WebTextCheckingResult>* webResults)
+{
+    Vector<WebTextCheckingResult> results;
+    if (mask & WebTextCheckingTypeSpelling) {
+        size_t offset = 0;
+        size_t length = text.length();
+        const WebUChar* data = ""
+        while (offset < length) {
+            int misspelledPosition = 0;
+            int misspelledLength = 0;
+            m_spellcheck.spellCheckWord(WebString(&data[offset], length - offset), &misspelledPosition, &misspelledLength);
+            if (!misspelledLength)
+                break;
+            WebTextCheckingResult result;
+            result.type = WebTextCheckingTypeSpelling;
+            result.location = offset + misspelledPosition;
+            result.length = misspelledLength;
+            results.append(result);
+            offset += misspelledPosition + misspelledLength;
+        }
+    }
+    if (mask & WebTextCheckingTypeGrammar)
+        MockGrammarCheck::checkGrammarOfString(text, &results);
+    webResults->assign(results);
+}
+
 void WebViewHost::requestCheckingOfText(const WebString& text, WebTextCheckingCompletion* completion)
 {
     if (text.isEmpty()) {

Modified: trunk/Tools/DumpRenderTree/chromium/WebViewHost.h (120422 => 120423)


--- trunk/Tools/DumpRenderTree/chromium/WebViewHost.h	2012-06-15 08:36:06 UTC (rev 120422)
+++ trunk/Tools/DumpRenderTree/chromium/WebViewHost.h	2012-06-15 08:41:30 UTC (rev 120423)
@@ -136,6 +136,7 @@
 
     // WebKit::WebSpellCheckClient
     virtual void spellCheck(const WebKit::WebString&, int& offset, int& length, WebKit::WebVector<WebKit::WebString>* optionalSuggestions);
+    virtual void checkTextOfParagraph(const WebKit::WebString&, WebKit::WebTextCheckingTypeMask, WebKit::WebVector<WebKit::WebTextCheckingResult>*);
     virtual void requestCheckingOfText(const WebKit::WebString&, WebKit::WebTextCheckingCompletion*);
     virtual WebKit::WebString autoCorrectWord(const WebKit::WebString&);
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to