Reviewed by Adam Roben
authorsullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Oct 2006 14:18:37 +0000 (14:18 +0000)
committersullivan <sullivan@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Oct 2006 14:18:37 +0000 (14:18 +0000)
        - fixes <rdar://problem/4804614> Bad grammar ranges are not visibly marked

        This patch introduces much of the guts of grammar checking, though still not enough to actually
        check grammar sensibly, due to:

        <rdar://problem/4811175> Many false reports of bad grammar appear, caused by insufficient
        context passed to grammar checker

        * platform/Logging.h:
        * platform/Logging.cpp:
        new log channel SpellingAndGrammar

        * bridge/mac/WebCorePageBridge.mm:
        (initializeLoggingChannelsIfNecessary):
        initialize new log channel

        * bridge/mac/FrameMac.mm:
        (WebCore::FrameMac::advanceToNextMisspelling):
        Compute bad grammar range when computing misspelling range. Find first detailed grammar range from the
        set NSSpellChecker determines. Compare it with misspelling range to see which is earliest (or shortest
        in the event of a tie), and do further processing with that one (select range; create marker that
        causes range to be visibly marked with a funky underline; update spelling panel appropriately).
        (WebCore::FrameMac::markMisspellings):
        More or less the same types of changes as in advanceToNextMisspelling The loops are structured just
        differently enough to make sharing code between these two functions a little tricky, so I decided to
        save that for a later patch.

        (WebCore::FrameMac::respondToChangedSelection):
        remove grammar markers when we remove spelling markers

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

WebCore/ChangeLog
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/bridge/mac/FrameMac.mm
WebCore/bridge/mac/WebCorePageBridge.mm
WebCore/platform/Logging.cpp
WebCore/platform/Logging.h

index 2c30f3822237ec5a717829a9380128416f4d0663..ce0a6d062ab636b84e01ffd6048da62b55c199e5 100644 (file)
@@ -1,3 +1,37 @@
+2006-10-30  John Sullivan  <sullivan@apple.com>
+
+        Reviewed by Adam Roben
+        
+        - fixes <rdar://problem/4804614> Bad grammar ranges are not visibly marked
+        
+        This patch introduces much of the guts of grammar checking, though still not enough to actually
+        check grammar sensibly, due to:
+          
+        <rdar://problem/4811175> Many false reports of bad grammar appear, caused by insufficient 
+        context passed to grammar checker
+
+        * platform/Logging.h:
+        * platform/Logging.cpp:
+        new log channel SpellingAndGrammar
+
+        * bridge/mac/WebCorePageBridge.mm:
+        (initializeLoggingChannelsIfNecessary):
+        initialize new log channel
+
+        * bridge/mac/FrameMac.mm:
+        (WebCore::FrameMac::advanceToNextMisspelling):
+        Compute bad grammar range when computing misspelling range. Find first detailed grammar range from the
+        set NSSpellChecker determines. Compare it with misspelling range to see which is earliest (or shortest
+        in the event of a tie), and do further processing with that one (select range; create marker that
+        causes range to be visibly marked with a funky underline; update spelling panel appropriately).
+        (WebCore::FrameMac::markMisspellings):
+        More or less the same types of changes as in advanceToNextMisspelling The loops are structured just 
+        differently enough to make sharing code between these two functions a little tricky, so I decided to 
+        save that for a later patch.
+        
+        (WebCore::FrameMac::respondToChangedSelection):
+        remove grammar markers when we remove spelling markers        
+
 2006-10-31  Nikolas Zimmermann <zimmermann@kde.org>
 
         Reviewed by Mitz.
index 9f8bed84eecdaaf84392f1a327116e88f46fd714..2501018cd4877660b058415eef00463f68992c5e 100644 (file)
                0867D690FE84028FC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
+                       compatibilityVersion = "Xcode 2.4";
                        hasScannedForEncodings = 1;
                        knownRegions = (
                                English,
                        mainGroup = 0867D691FE84028FC02AAC07 /* WebKit */;
                        productRefGroup = 034768DFFF38A50411DB9C8B /* Products */;
                        projectDirPath = "";
+                       projectRoot = "";
+                       shouldCheckCompatibility = 1;
                        targets = (
                                93F198A508245E59001E9ABC /* WebCore */,
                                DD041FBE09D9DDBE0010AF2A /* Derived Sources */,
index 472941705cc38ab565efeeeb020f59737696cc1c..1ac9a9c610e904aa0c510d618af79b7eaec76354 100644 (file)
 - (NPObject *)createPluginScriptableObject;
 @end
 
+#ifndef BUILDING_ON_TIGER
+@interface NSSpellChecker (UpcomingAPI)
+- (void)updateSpellingPanelWithGrammarString:(NSString *)grammarString detail:(NSDictionary *)grammarDetail;
+@end
+#endif
+
 using namespace std;
 using namespace KJS::Bindings;
 
@@ -634,26 +640,79 @@ void FrameMac::advanceToNextMisspelling(bool startBeforeSelection)
             if (len > 1 || !DeprecatedChar(chars[0]).isSpace()) {
                 NSString *chunk = [[NSString alloc] initWithCharactersNoCopy:const_cast<UChar*>(chars) length:len freeWhenDone:NO];
                 NSRange misspellingNSRange = [checker checkSpellingOfString:chunk startingAt:0 language:nil wrap:NO inSpellDocumentWithTag:editor()->client()->spellCheckerDocumentTag() wordCount:NULL];
-                [chunk release];
-                if (misspellingNSRange.length == 0) {
+
+                NSDictionary *grammarDetail = nil;
+                NSRange badGrammarNSRange = NSMakeRange(NSNotFound, 0);
+                NSRange grammarDetailNSRange = NSMakeRange(NSNotFound, 0);
+#ifndef BUILDING_ON_TIGER
+                if (editor()->client()->isGrammarCheckingEnabled()) {
+                    NSArray *grammarDetails = nil;
+                    // FIXME 4811175: grammar checking needs entire sentences (at least) to operate correctly. This code currently only
+                    // passes whatever chunk was computed for spell checking, which in most cases is not large enough and causes
+                    // spurious complaints of bad grammar.
+                    badGrammarNSRange = [checker checkGrammarOfString:chunk startingAt:0 language:nil wrap:NO inSpellDocumentWithTag:editor()->client()->spellCheckerDocumentTag() details:&grammarDetails];
+                    LOG(SpellingAndGrammar, "checked chunk \'%@\' starting at index %d, bad grammar range is %@', details:%@", chunk, 0, NSStringFromRange(badGrammarNSRange), grammarDetails);
+
+                    // The grammar API allows for multiple details dictionaries for each range of questionable grammar.
+                    // Iterate through them to find the one whose detailRange is earliest (or shortest in case of tie).
+                    // The detailRange represents the small fragment (e.g., single word) that could be changed to make
+                    // the badGrammarNSRange (e.g., complete sentence) correct.
+                    for (NSDictionary *detail in grammarDetails) {
+                        NSValue *rangeAsNSValue = [detail objectForKey:NSGrammarRange];
+                        NSRange detailRange = [rangeAsNSValue rangeValue];
+                        ASSERT(detailRange.length > 0);
+                        detailRange.location += badGrammarNSRange.location;
+                        
+                        // Remember this detail if it's the first one, or if it starts earlier than the remembered one, or if it starts at the
+                        // same location as the remembered one but ends sooner.
+                        if (!grammarDetail || detailRange.location < grammarDetailNSRange.location 
+                            || (detailRange.location == grammarDetailNSRange.location && detailRange.length < grammarDetailNSRange.length)) {
+                            grammarDetail = detail;
+                            grammarDetailNSRange = detailRange;
+                        }
+                    }
+                }
+#endif
+                
+                bool hasMisspelling = misspellingNSRange.length > 0;
+                bool hasBadGrammar = badGrammarNSRange.length > 0;
+                
+                // If we found questionable grammar, we should have a detail dictionary and a non-empty detail range.
+                // If we didn't find questionable grammar, we should have no detail dictionary and an empty detail range.
+                ASSERT(hasBadGrammar == (grammarDetail != nil));
+                ASSERT(hasBadGrammar == (grammarDetailNSRange.length > 0));
+                
+                if (!hasMisspelling && !hasBadGrammar) {
                     it.advance();
+                    [chunk release];
                     continue;
                 }
 
-                // Build up result range and string.  Note the misspelling may span many text nodes,
-                // but the CharIterator insulates us from this complexity
-                RefPtr<Range> misspellingRange(rangeOfContents(document()));
+                // If we found both a misspelling and some questionable grammar, we only want to pay attention to the first
+                // of these two ranges. If they match exactly we'll treat it as a misspelling.
+                BOOL markMisspelling = !hasBadGrammar || (misspellingNSRange.location < grammarDetailNSRange.location) 
+                    || (misspellingNSRange.location == grammarDetailNSRange.location && misspellingNSRange.length <= grammarDetailNSRange.length);
+                
+                // Build up result range and string.  Note the bad range may span many text nodes,
+                // but the CharIterator insulates us from this complexity.
+                NSRange badNSRangeToMark = markMisspelling ? misspellingNSRange : grammarDetailNSRange;
+                RefPtr<Range> badRangeToMark(rangeOfContents(document()));
                 CharacterIterator chars(it.range().get());
-                chars.advance(misspellingNSRange.location);
-                misspellingRange->setStart(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
-                DeprecatedString result = chars.string(misspellingNSRange.length);
-                misspellingRange->setEnd(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
-                selectionController()->setSelection(Selection(misspellingRange.get(), DOWNSTREAM));
+                chars.advance(badNSRangeToMark.location);
+                badRangeToMark->setStart(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
+                chars.advance(badNSRangeToMark.length);
+                badRangeToMark->setEnd(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
+                selectionController()->setSelection(Selection(badRangeToMark.get(), DOWNSTREAM));
                 revealSelection();
                 // Mark misspelling in document.
-                document()->addMarker(misspellingRange.get(), DocumentMarker::Spelling);
+                document()->addMarker(badRangeToMark.get(), markMisspelling ? DocumentMarker::Spelling : DocumentMarker::Grammar);
+                
+                if (markMisspelling)
+                    [checker updateSpellingPanelWithMisspelledWord:[chunk substringWithRange:misspellingNSRange]];
+                else
+                    [checker updateSpellingPanelWithGrammarString:[chunk substringWithRange:badGrammarNSRange] detail:grammarDetail];
                 
-                [checker updateSpellingPanelWithMisspelledWord:result];
+                [chunk release];
                 return;
             }
         }
@@ -2476,24 +2535,72 @@ void FrameMac::markMisspellings(const Selection& selection)
         if (len > 1 || !DeprecatedChar(chars[0]).isSpace()) {
             NSString *chunk = [[NSString alloc] initWithCharactersNoCopy:const_cast<UChar*>(chars) length:len freeWhenDone:NO];
             int startIndex = 0;
-            // Loop over the chunk to find each misspelling in it.
+            // Loop over the chunk to find each misspelled word or instance of questionable grammar in it.
             while (startIndex < len) {
                 NSRange misspellingNSRange = [checker checkSpellingOfString:chunk startingAt:startIndex language:nil wrap:NO inSpellDocumentWithTag:editor()->client()->spellCheckerDocumentTag() wordCount:NULL];
-                if (misspellingNSRange.length == 0)
+
+                NSDictionary *grammarDetail = nil;
+                NSRange badGrammarNSRange = NSMakeRange(NSNotFound, 0);
+                NSRange grammarDetailNSRange = NSMakeRange(NSNotFound, 0);
+#ifndef BUILDING_ON_TIGER
+                if (editor()->client()->isGrammarCheckingEnabled()) {
+                    NSArray *grammarDetails = nil;
+                    // FIXME 4811175: grammar checking needs entire sentences (at least) to operate correctly. This code currently only
+                    // passes whatever chunk was computed for spell checking, which in most cases is not large enough and causes
+                    // spurious complaints of bad grammar.
+                    badGrammarNSRange = [checker checkGrammarOfString:chunk startingAt:startIndex language:nil wrap:NO inSpellDocumentWithTag:editor()->client()->spellCheckerDocumentTag() details:&grammarDetails];
+                    LOG(SpellingAndGrammar, "checked chunk \'%@\' starting at index %d, bad grammar range is %@', details:%@", chunk, startIndex, NSStringFromRange(badGrammarNSRange), grammarDetails);
+                    
+                    // The grammar API allows for multiple details dictionaries for each range of questionable grammar.
+                    // Iterate through them to find the one whose detailRange is earliest (or shortest in case of tie).
+                    // The detailRange represents the small fragment (e.g., single word) that could be changed to make
+                    // the badGrammarNSRange (e.g., complete sentence) correct.
+                    for (NSDictionary *detail in grammarDetails) {
+                        NSValue *rangeAsNSValue = [detail objectForKey:NSGrammarRange];
+                        NSRange detailRange = [rangeAsNSValue rangeValue];
+                        ASSERT(detailRange.length > 0);
+                        detailRange.location += badGrammarNSRange.location;
+                        
+                        // Remember this detail if it's the first one, or if it starts earlier than the remembered one, or if it starts at the
+                        // same location as the remembered one but ends sooner.
+                        if (!grammarDetail || detailRange.location < grammarDetailNSRange.location 
+                            || (detailRange.location == grammarDetailNSRange.location && detailRange.length < grammarDetailNSRange.length)) {
+                            grammarDetail = detail;
+                            grammarDetailNSRange = detailRange;
+                        }
+                    }
+                }
+#endif
+
+                bool hasMisspelling = misspellingNSRange.length > 0;
+                bool hasBadGrammar = badGrammarNSRange.length > 0;
+                
+                // If we found questionable grammar, we should have a detail dictionary and a non-empty detail range.
+                // If we didn't find questionable grammar, we should have no detail dictionary and an empty detail range.
+                ASSERT(hasBadGrammar == (grammarDetail != nil));
+                ASSERT(hasBadGrammar == (grammarDetailNSRange.length > 0));
+                
+                if (!hasMisspelling && !hasBadGrammar)
                     break;
 
-                // Build up result range and string.  Note the misspelling may span many text nodes,
-                // but the CharIterator insulates us from this complexity
-                RefPtr<Range> misspellingRange(rangeOfContents(document()));
+                // If we found both a misspelling and some questionable grammar, we only want to pay attention to the first
+                // of these two ranges. If they match exactly we'll treat it as a misspelling.
+                BOOL markMisspelling = !hasBadGrammar || (misspellingNSRange.location < grammarDetailNSRange.location) 
+                    || (misspellingNSRange.location == grammarDetailNSRange.location && misspellingNSRange.length <= grammarDetailNSRange.length);
+                
+                // Build up result range and string.  Note the bad range may span many text nodes,
+                // but the CharIterator insulates us from this complexity.
+                NSRange badNSRangeToMark = markMisspelling ? misspellingNSRange : grammarDetailNSRange;
+                RefPtr<Range> badRangeToMark(rangeOfContents(document()));
                 CharacterIterator chars(it.range().get());
-                chars.advance(misspellingNSRange.location);
-                misspellingRange->setStart(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
-                chars.advance(misspellingNSRange.length);
-                misspellingRange->setEnd(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
+                chars.advance(badNSRangeToMark.location);
+                badRangeToMark->setStart(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
+                chars.advance(badNSRangeToMark.length);
+                badRangeToMark->setEnd(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
                 // Mark misspelling in document.
-                document()->addMarker(misspellingRange.get(), DocumentMarker::Spelling);
+                document()->addMarker(badRangeToMark.get(), markMisspelling ? DocumentMarker::Spelling : DocumentMarker::Grammar);
                 
-                startIndex = misspellingNSRange.location + misspellingNSRange.length;
+                startIndex = badNSRangeToMark.location + badNSRangeToMark.length;
             }
             [chunk release];
         }
@@ -2525,9 +2632,12 @@ void FrameMac::respondToChangedSelection(const Selection &oldSelection, bool clo
             // This only erases a marker in the first word of the selection.
             // Perhaps peculiar, but it matches AppKit.
             document()->removeMarkers(newAdjacentWords.toRange().get(), DocumentMarker::Spelling);
-        } else
-            // When continuous spell checking is off, no markers appear after the selection changes.
+            document()->removeMarkers(newAdjacentWords.toRange().get(), DocumentMarker::Grammar);
+        } else {
+            // When continuous spell checking is off, existing markers disappear after the selection changes.
             document()->removeMarkers(DocumentMarker::Spelling);
+            document()->removeMarkers(DocumentMarker::Grammar);
+        }
     }
 
     [_bridge respondToChangedSelection];
index d2d9e5d874757c336fc4f026a13895f843a67fb3..44d917696169fe47df5fba7ec2cd01906071ed6b 100644 (file)
@@ -64,6 +64,7 @@ static void initializeLoggingChannelsIfNecessary()
     initializeLogChannel(LogTextConversion);
     initializeLogChannel(LogIconDatabase);
     initializeLogChannel(LogSQLDatabase);
+    initializeLogChannel(LogSpellingAndGrammar);
 }
 
 - (id)init
index 0264eed089f90f32fe54616785b058f35ea9a9a9..8ad7bb54f1b765e314df573ce0716685d82b8938 100644 (file)
@@ -42,4 +42,6 @@ WTFLogChannel LogTextConversion =    { 0x00000200, "WebCoreLogLevel", WTFLogChan
 WTFLogChannel LogIconDatabase =      { 0x00000400, "WebCoreLogLevel", WTFLogChannelOff };
 WTFLogChannel LogSQLDatabase =       { 0x00000800, "WebCoreLogLevel", WTFLogChannelOff };
 
+WTFLogChannel LogSpellingAndGrammar ={ 0x00001000, "WebCoreLogLevel", WTFLogChannelOff };
+
 }
index 8a397588294bd56aae7703f36b87d094d559f7c3..b0baa46d63bb8479dc8173239473078afe779900 100644 (file)
@@ -44,6 +44,7 @@ namespace WebCore {
     extern WTFLogChannel LogTextConversion;
     extern WTFLogChannel LogIconDatabase;
     extern WTFLogChannel LogSQLDatabase;
+    extern WTFLogChannel LogSpellingAndGrammar;
 }
 
 #endif