Reviewed by Tim Hatcher
authorsullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Apr 2007 16:28:40 +0000 (16:28 +0000)
committersullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Apr 2007 16:28:40 +0000 (16:28 +0000)
        - fixed <rdar://problem/4859132> Grammar must always be checked in entire-sentence chunks,
          and shouldn't show markers for current sentence

        * editing/Editor.h:
        add markBadGrammar, now distinct from markMisspellings

        * editing/mac/EditorMac.mm:
        (WebCore::Editor::markMisspellingsAfterTypingToPosition):
        call markMisspellings on one word, and markBadGrammar on entire sentence
        (WebCore::markMisspellingsOrBadGrammar):
        new static function, extracted from markMisspellings
        (WebCore::Editor::markMisspellings):
        now calls extracted function
        (WebCore::Editor::markBadGrammar):
        new method, calls extracted function

        * page/Frame.cpp:
        (WebCore::Frame::respondToChangedSelection):
        update grammar markers for entire new and old sentences

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

WebCore/ChangeLog
WebCore/editing/Editor.h
WebCore/editing/mac/EditorMac.mm
WebCore/page/Frame.cpp

index ceea6f6..9bd6ff1 100644 (file)
@@ -1,3 +1,27 @@
+2007-04-10  John Sullivan  <sullivan@apple.com>
+
+        Reviewed by Tim Hatcher
+
+        - fixed <rdar://problem/4859132> Grammar must always be checked in entire-sentence chunks, 
+          and shouldn't show markers for current sentence
+
+        * editing/Editor.h:
+        add markBadGrammar, now distinct from markMisspellings
+
+        * editing/mac/EditorMac.mm:
+        (WebCore::Editor::markMisspellingsAfterTypingToPosition):
+        call markMisspellings on one word, and markBadGrammar on entire sentence
+        (WebCore::markMisspellingsOrBadGrammar):
+        new static function, extracted from markMisspellings
+        (WebCore::Editor::markMisspellings):
+        now calls extracted function
+        (WebCore::Editor::markBadGrammar):
+        new method, calls extracted function
+
+        * page/Frame.cpp:
+        (WebCore::Frame::respondToChangedSelection):
+        update grammar markers for entire new and old sentences
+
 2007-04-09  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by John.
index 2ea0eef..0f6a01f 100644 (file)
@@ -159,6 +159,7 @@ public:
     Vector<String> guessesForUngrammaticalSelection();
     void markMisspellingsAfterTypingToPosition(const VisiblePosition&);
     void markMisspellings(const Selection&);
+    void markBadGrammar(const Selection&);
     void advanceToNextMisspelling(bool startBeforeSelection = false);
     void showSpellingGuessPanel();
     bool spellingPanelIsShowing();
index ce8fbc3..351754b 100644 (file)
@@ -573,12 +573,14 @@ void Editor::markMisspellingsAfterTypingToPosition(const VisiblePosition &p)
     if (!isContinuousSpellCheckingEnabled())
         return;
     
-    // FIXME 4859132: We currently only check the adjacent words. This is fine for spelling, where each
-    // misspelling is fully contained within a word. But this isn't good enough for grammar checking,
-    // where more context must be taken into account. We'll have to reconsider the grammar in the entire
-    // "conceptual grammar block", which is not well-defined but must be at least a sentence long, and
-    // perhaps a paragraph in some languages.
+    // Check spelling of one word
     markMisspellings(Selection(startOfWord(p, LeftWordIfOnBoundary), endOfWord(p, RightWordIfOnBoundary)));
+    
+    if (!isGrammarCheckingEnabled())
+        return;
+    
+    // Check grammar of entire sentence
+    markBadGrammar(Selection(startOfSentence(p), endOfSentence(p)));
 }
 
 static void markAllMisspellingsInRange(NSSpellChecker *checker, int tag, Range* searchRange)
@@ -599,13 +601,15 @@ static void markAllBadGrammarInRange(NSSpellChecker *checker, int tag, Range* se
     findFirstBadGrammarInRange(checker, tag, searchRange, ignoredGrammarDetail, ignoredOffset, true);
 }
 #endif
-
-void Editor::markMisspellings(const Selection& selection)
+    
+static void markMisspellingsOrBadGrammar(Editor* editor, const Selection& selection, bool checkSpelling)
 {
     // This function is called with a selection already expanded to word boundaries.
     // Might be nice to assert that here.
     
-    if (!isContinuousSpellCheckingEnabled())
+    // This function is used only for as-you-type checking, so if that's off we do nothing. Note that
+    // grammar checking can only be on if spell checking is also on.
+    if (!editor->isContinuousSpellCheckingEnabled())
         return;
     
     RefPtr<Range> searchRange(selection.toRange());
@@ -623,11 +627,27 @@ void Editor::markMisspellings(const Selection& selection)
     if (checker == nil)
         return;
     
-    markAllMisspellingsInRange(checker, spellCheckerDocumentTag(), searchRange.get());
+    if (checkSpelling)
+        markAllMisspellingsInRange(checker, editor->spellCheckerDocumentTag(), searchRange.get());
+    else {
+#ifdef BUILDING_ON_TIGER
+        ASSERT_NOT_REACHED();
+#else
+        if (editor->isGrammarCheckingEnabled())
+            markAllBadGrammarInRange(checker, editor->spellCheckerDocumentTag(), searchRange.get());
+#endif
+    }    
+}
+
+void Editor::markMisspellings(const Selection& selection)
+{
+    markMisspellingsOrBadGrammar(this, selection, true);
+}
     
+void Editor::markBadGrammar(const Selection& selection)
+{
 #ifndef BUILDING_ON_TIGER
-    if (isGrammarCheckingEnabled())
-        markAllBadGrammarInRange(checker, spellCheckerDocumentTag(), searchRange.get());
+    markMisspellingsOrBadGrammar(this, selection, false);
 #endif
 }
 
index 6026626..ca7d303 100644 (file)
@@ -1818,25 +1818,32 @@ void Frame::respondToChangedSelection(const Selection &oldSelection, bool closeT
     if (document()) {
         if (editor()->isContinuousSpellCheckingEnabled()) {
             Selection oldAdjacentWords;
+            Selection oldSelectedSentence;
             
             // If this is a change in selection resulting from a delete operation, oldSelection may no longer
             // be in the document.
             if (oldSelection.start().node() && oldSelection.start().node()->inDocument()) {
                 VisiblePosition oldStart(oldSelection.visibleStart());
                 oldAdjacentWords = Selection(startOfWord(oldStart, LeftWordIfOnBoundary), endOfWord(oldStart, RightWordIfOnBoundary));   
+                oldSelectedSentence = Selection(startOfSentence(oldStart), endOfSentence(oldStart));   
             }
             
             VisiblePosition newStart(selectionController()->selection().visibleStart());
             Selection newAdjacentWords(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
+            Selection newSelectedSentence(startOfSentence(newStart), endOfSentence(newStart));
             
             // When typing we check spelling elsewhere, so don't redo it here.
-            if (closeTyping && oldAdjacentWords != newAdjacentWords)
+            if (closeTyping && oldAdjacentWords != newAdjacentWords) {
                 editor()->markMisspellings(oldAdjacentWords);
+                
+                if (oldSelectedSentence != newSelectedSentence)
+                    editor()->markBadGrammar(oldSelectedSentence);
+            }
             
-            // This only erases a marker in the first word of the selection.
+            // This only erases a marker in the first unit (word or sentence) of the selection.
             // Perhaps peculiar, but it matches AppKit.
             document()->removeMarkers(newAdjacentWords.toRange().get(), DocumentMarker::Spelling);
-            document()->removeMarkers(newAdjacentWords.toRange().get(), DocumentMarker::Grammar);
+            document()->removeMarkers(newSelectedSentence.toRange().get(), DocumentMarker::Grammar);
         } else {
             // When continuous spell checking is off, existing markers disappear after the selection changes.
             document()->removeMarkers(DocumentMarker::Spelling);