Changeset 17508 in webkit

Timestamp:

Oct 31, 2006, 6:49:02 PM (18 years ago)

Author:

sullivan

Message:

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.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bridge/mac/FrameMac.mm (modified) (4 diffs)
• dom/Document.cpp (modified) (3 diffs)
• dom/Document.h (modified) (2 diffs)
• dom/DocumentMarker.h (modified) (2 diffs)
• rendering/HitTestResult.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/WebCore/bridge/mac/FrameMac.mm

@@ -635 +635 @@
     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
@@ -708 +712 @@
                 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
                 
@@ -2531 +2537 @@
     
     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
@@ -2602 +2612 @@
                 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;
             }

trunk/WebCore/dom/Document.cpp

@@ -2665 +2665 @@
 }
 
-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.
@@ -2671 +2671 @@
         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);
     }
@@ -2858 +2858 @@
 }
 
+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)
 {

trunk/WebCore/dom/Document.h

@@ -531 +531 @@
     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);
@@ -543 +543 @@
     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);

trunk/WebCore/dom/DocumentMarker.h

@@ -24 +24 @@
 #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 {
 
@@ -39 +43 @@
     unsigned startOffset;
     unsigned endOffset;
+    String description;
 
     bool operator==(const DocumentMarker& o) const

trunk/WebCore/rendering/HitTestResult.cpp

@@ -119 +119 @@
 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();
 }