Reviewed by Maciej

        - fixed <rdar://problem/4804627> ToolTips do not appear for grammar suggestions

        The foundation of this was in my last checkin. This checkin is all about displaying
        the correct string in the toolTip.

        * dom/DocumentMarker.h:
        New description field in this struct.

        * bridge/mac/FrameMac.mm:
        (WebCore::FrameMac::advanceToNextMisspelling):
        When adding a grammar marker, supply the appropriate description. Also, added a comment
        about the remaining work to make grammar checking return sensible answers.
        (WebCore::FrameMac::markMisspellings):
        ditto (yes, still needs some refactoring to minimize duplicated code)

        * dom/Document.h:
        * dom/Document.cpp:
        (WebCore::Document::addMarker):
        Now takes an optional description string
        (WebCore::Document::markerContainingPoint):
        New function, returns a pointer to the (first) marker of the specified type whose rect
        contains the specified point, or 0 if none.

        * rendering/HitTestResult.cpp:
        (WebCore::HitTestResult::spellingToolTip):
        Replaced hardwired string placeholder implementation with code that uses markerContainingPoint
        and gets the description from the marker.

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

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index e2cddaa66e31d8dd1f9b1ccc6b4ab0647912226e..570dfc1b8dbaac52037a6efe9704adc1be33be01 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2006-10-31  John Sullivan  <sullivan@apple.com>
+
+        Reviewed by Maciej
+        
+        - fixed <rdar://problem/4804627> ToolTips do not appear for grammar suggestions
+        
+        The foundation of this was in my last checkin. This checkin is all about displaying
+        the correct string in the toolTip.
+
+        * dom/DocumentMarker.h:
+        New description field in this struct.
+
+        * bridge/mac/FrameMac.mm:
+        (WebCore::FrameMac::advanceToNextMisspelling):
+        When adding a grammar marker, supply the appropriate description. Also, added a comment
+        about the remaining work to make grammar checking return sensible answers.
+        (WebCore::FrameMac::markMisspellings):
+        ditto (yes, still needs some refactoring to minimize duplicated code)
+        
+        * dom/Document.h:
+        * dom/Document.cpp:
+        (WebCore::Document::addMarker):
+        Now takes an optional description string
+        (WebCore::Document::markerContainingPoint):
+        New function, returns a pointer to the (first) marker of the specified type whose rect 
+        contains the specified point, or 0 if none.
+        
+        * rendering/HitTestResult.cpp:
+        (WebCore::HitTestResult::spellingToolTip):
+        Replaced hardwired string placeholder implementation with code that uses markerContainingPoint
+        and gets the description from the marker.
+
 2006-10-31  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Maciej.

diff --git a/WebCore/bridge/mac/FrameMac.mm b/WebCore/bridge/mac/FrameMac.mm
index 8beb84a34ff11c442d769cab342b56fd450659a7..21128289ddbf027dbc660dc93b73bc85701535b3 100644 (file)
--- a/WebCore/bridge/mac/FrameMac.mm
+++ b/WebCore/bridge/mac/FrameMac.mm
@@ -634,6 +634,10 @@ void FrameMac::advanceToNextMisspelling(bool startBeforeSelection)
     Node *searchEndAfterWrapNode = it.range()->endContainer(exception);
     int searchEndAfterWrapOffset = it.range()->endOffset(exception);
 
+    // FIXME: We need to compute the entire paragraph(s?) containing the search range, and use that as "chunk" below,
+    // so there's enough context for the spell checker to evaluate grammar (4811175) and in some cases even spelling (4149250).
+    // That means we need to compute the right offset to pass to the NSSpellChecker methods.
+
     while (1) {
         if (!it.atEnd()) {      // we may be starting at the end of the doc, and already by atEnd
             const UChar* chars = it.characters();
@@ -707,14 +711,16 @@ void FrameMac::advanceToNextMisspelling(bool startBeforeSelection)
                 badRangeToMark->setEnd(chars.range()->startContainer(exception), chars.range()->startOffset(exception), exception);
                 selectionController()->setSelection(Selection(badRangeToMark.get(), DOWNSTREAM));
                 revealSelection();
-                // Mark misspelling in document.
-                document()->addMarker(badRangeToMark.get(), markMisspelling ? DocumentMarker::Spelling : DocumentMarker::Grammar);
                 
-                if (markMisspelling)
+                if (markMisspelling) {
                     [checker updateSpellingPanelWithMisspelledWord:[chunk substringWithRange:misspellingNSRange]];
+                    document()->addMarker(badRangeToMark.get(), DocumentMarker::Spelling);
+                }
 #ifndef BUILDING_ON_TIGER
-                else
+                else {
                     [checker updateSpellingPanelWithGrammarString:[chunk substringWithRange:badGrammarNSRange] detail:grammarDetail];
+                    document()->addMarker(badRangeToMark.get(), DocumentMarker::Grammar, [grammarDetail objectForKey:NSGrammarUserDescription]);
+                }
 #endif
                 
                 [chunk release];
@@ -2531,6 +2537,10 @@ void FrameMac::markMisspellings(const Selection& selection)
     
     WordAwareIterator it(searchRange.get());
     
+    // FIXME: We need to compute the entire paragraph(s?) containing the search range, and use that as "chunk" below,
+    // so there's enough context for the spell checker to evaluate grammar (4811175) and in some cases even spelling (4149250).
+    // That means we need to compute the right offset to pass to the NSSpellChecker methods.
+    
     while (!it.atEnd()) {      // we may be starting at the end of the doc, and already by atEnd
         const UChar* chars = it.characters();
         int len = it.length();
@@ -2601,9 +2611,11 @@ void FrameMac::markMisspellings(const Selection& selection)
                 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(badRangeToMark.get(), markMisspelling ? DocumentMarker::Spelling : DocumentMarker::Grammar);
-                
+                if (markMisspelling)
+                    document()->addMarker(badRangeToMark.get(), DocumentMarker::Spelling);
+                else
+                    document()->addMarker(badRangeToMark.get(), DocumentMarker::Grammar, [grammarDetail objectForKey:NSGrammarUserDescription]);
+
                 startIndex = badNSRangeToMark.location + badNSRangeToMark.length;
             }
             [chunk release];

diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
index 64c5148d3cc75b0fe8d7c5c8aa4c62d1a118001e..7a980f197a3a7ac4f940a9b3fe9b48cfcc6e5ba5 100644 (file)
--- a/WebCore/dom/Document.cpp
+++ b/WebCore/dom/Document.cpp
@@ -2664,13 +2664,13 @@ static IntRect placeholderRectForMarker(void)
     return IntRect(-1,-1,-1,-1);
 }
 
-void Document::addMarker(Range *range, DocumentMarker::MarkerType type)
+void Document::addMarker(Range *range, DocumentMarker::MarkerType type, String description)
 {
     // Use a TextIterator to visit the potentially multiple nodes the range covers.
     for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) {
         RefPtr<Range> textPiece = markedText.range();
         int exception = 0;
-        DocumentMarker marker = {type, textPiece->startOffset(exception), textPiece->endOffset(exception)};
+        DocumentMarker marker = {type, textPiece->startOffset(exception), textPiece->endOffset(exception), description};
         addMarker(textPiece->startContainer(exception), marker);
     }
 }
@@ -2857,6 +2857,38 @@ void Document::removeMarkers(Node *node, unsigned startOffset, int length, Docum
         node->renderer()->repaint();
 }
 
+DocumentMarker* Document::markerContainingPoint(const IntPoint& point, DocumentMarker::MarkerType markerType)
+{
+    // outer loop: process each node that contains any markers
+    MarkerMap::iterator end = m_markers.end();
+    for (MarkerMap::iterator nodeIterator = m_markers.begin(); nodeIterator != end; ++nodeIterator) {
+        // inner loop; process each marker in this node
+        MarkerMapVectorPair* vectorPair = nodeIterator->second;
+        Vector<DocumentMarker>& markers = vectorPair->first;
+        Vector<IntRect>& rects = vectorPair->second;
+        ASSERT(markers.size() == rects.size());
+        unsigned markerCount = markers.size();
+        for (unsigned markerIndex = 0; markerIndex < markerCount; ++markerIndex) {
+            DocumentMarker& marker = markers[markerIndex];
+            
+            // skip marker that is wrong type
+            if (marker.type != markerType && markerType != DocumentMarker::AllMarkers)
+                continue;
+            
+            IntRect& r = rects[markerIndex];
+            
+            // skip placeholder rects
+            if (r == placeholderRectForMarker())
+                continue;
+            
+            if (r.contains(point))
+                return &marker;
+        }
+    }
+    
+    return 0;
+}
+
 Vector<DocumentMarker> Document::markersForNode(Node* node)
 {
     MarkerMapVectorPair* vectorPair = m_markers.get(node);

diff --git a/WebCore/dom/Document.h b/WebCore/dom/Document.h
index d8ff25400746293174e05beec02b78cc40be26ac..2092293c4c3d91227bc130239fc3ed5488b65bf1 100644 (file)
--- a/WebCore/dom/Document.h
+++ b/WebCore/dom/Document.h
@@ -530,7 +530,7 @@ public:
     bool queryCommandSupported(const String& command);
     String queryCommandValue(const String& command);
     
-    void addMarker(Range*, DocumentMarker::MarkerType);
+    void addMarker(Range*, DocumentMarker::MarkerType, String description = String());
     void addMarker(Node*, DocumentMarker);
     void copyMarkers(Node *srcNode, unsigned startOffset, int length, Node *dstNode, int delta, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
     void removeMarkers(Range*, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
@@ -542,6 +542,7 @@ public:
     void invalidateRenderedRectsForMarkersInRect(const IntRect&);
     void shiftMarkers(Node*, unsigned startOffset, int delta, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
 
+    DocumentMarker* markerContainingPoint(const IntPoint&, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
     Vector<DocumentMarker> markersForNode(Node*);
     Vector<IntRect> renderedRectsForMarkers(DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
     

diff --git a/WebCore/dom/DocumentMarker.h b/WebCore/dom/DocumentMarker.h
index e8934b7000adddeba05d0f9e5f67e10ed1dbc6ba..13b2ef5e2f0d3386a681577a1b253506ff0f930b 100644 (file)
--- a/WebCore/dom/DocumentMarker.h
+++ b/WebCore/dom/DocumentMarker.h
@@ -23,9 +23,13 @@
 #ifndef DOM_DocumentMarker_h
 #define DOM_DocumentMarker_h
 
+#include "PlatformString.h"
+
 namespace WebCore {
+    class String;
 
-// A range of a node within a document that is "marked", such as being misspelled
+// A range of a node within a document that is "marked", such as the range of a misspelled word.
+// It optionally includes a description that could be displayed in the user interface.
 struct DocumentMarker {
 
     enum MarkerType {
@@ -38,6 +42,7 @@ struct DocumentMarker {
     MarkerType type;
     unsigned startOffset;
     unsigned endOffset;
+    String description;
 
     bool operator==(const DocumentMarker& o) const
     {

diff --git a/WebCore/rendering/HitTestResult.cpp b/WebCore/rendering/HitTestResult.cpp
index 154a84b924b00ae6d30183ad62e81da70f467b61..71eed4734d8e76f8b1b7b4c8db7fbc6cbc540341 100644 (file)
--- a/WebCore/rendering/HitTestResult.cpp
+++ b/WebCore/rendering/HitTestResult.cpp
@@ -118,16 +118,12 @@ bool HitTestResult::isSelected() const
 
 String HitTestResult::spellingToolTip() const
 {
-    // Return the tool tip string associated with this point
-    // FIXME: At the moment this returns the same hardwired string for all bad grammar rects. This needs to
-    // instead return an instance-specific string that was stashed away when the bad grammar was discovered.
-    // Checking it in now just to modularize the work a little.
-    Vector<IntRect> rects = m_innerNonSharedNode->document()->renderedRectsForMarkers(DocumentMarker::Grammar);
-    unsigned count = rects.size();
-    for (unsigned index = 0; index < count; ++index)
-        if (rects[index].contains(m_point))
-            return "Questionable grammar!";
-
+    // Return the tool tip string associated with this point, if any. Only markers associated with bad grammar
+    // currently supply strings, but maybe someday markers associated with misspelled words will also.
+    DocumentMarker* marker = m_innerNonSharedNode->document()->markerContainingPoint(m_point, DocumentMarker::Grammar);
+    if (marker)
+        return marker->description;
+    
     return String();
 }