2008-08-13 Beth Dakin <bdakin@apple.com>
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Aug 2008 21:36:15 +0000 (21:36 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Aug 2008 21:36:15 +0000 (21:36 +0000)
        Reviewed by Sam Weinig.

        Fix for <rdar://problem/6141345>

        This patch refines findString and markAllMatchesForText functions'
        interactions with disconnected frames. They no longer rely on
        knowing where a range is relative to the visible region and work
        with disconnected frames that contain frames.

        * editing/Editor.cpp:
        (WebCore::Editor::insideVisibleArea): Now returns a bool instead of
        the visiblity enum.
        (WebCore::Editor::firstVisibleRange): This now returns the very
        first visible range in the document. It's no longer dependent on
        searching forward.
        (WebCore::Editor::lastVisibleRange): This now returns the very last
        visible range in the document. It's no longer dependent on
        searching backwards.
        (WebCore::Editor::nextVisibleRange): This returns the next visible
        range in the appropriate direction from the current range.
        * editing/Editor.h:
        * page/Frame.cpp:
        (WebCore::Frame::findString):
        (WebCore::Frame::markAllMatchesForText):

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

WebCore/ChangeLog
WebCore/editing/Editor.cpp
WebCore/editing/Editor.h
WebCore/page/Frame.cpp

index 8d6d7fde6aa2b20e6eb7d2f07bc66a1b468bed15..2ee68e3700e23459a15a9540b0221db1e8c4df74 100644 (file)
@@ -1,3 +1,30 @@
+2008-08-13  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Sam Weinig.
+
+        Fix for <rdar://problem/6141345>
+
+        This patch refines findString and markAllMatchesForText functions' 
+        interactions with disconnected frames. They no longer rely on 
+        knowing where a range is relative to the visible region and work 
+        with disconnected frames that contain frames.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::insideVisibleArea): Now returns a bool instead of 
+        the visiblity enum.
+        (WebCore::Editor::firstVisibleRange): This now returns the very 
+        first visible range in the document. It's no longer dependent on 
+        searching forward.
+        (WebCore::Editor::lastVisibleRange): This now returns the very last 
+        visible range in the document. It's no longer dependent on 
+        searching backwards.
+        (WebCore::Editor::nextVisibleRange): This returns the next visible 
+        range in the appropriate direction from the current range.
+        * editing/Editor.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::findString):
+        (WebCore::Frame::markAllMatchesForText):
+
 2008-08-13  Kevin Ollivier  <kevino@theolliviers.com>
 
         wx build fix for case-sensitive platforms, like Linux.
index 0651830e0e6cd2a3a9b08b7c2633b8827a5a898d..a95987579ba40fb246c43e4c0c92b383aaaf0f16 100644 (file)
@@ -42,6 +42,7 @@
 #include "EventNames.h"
 #include "FocusController.h"
 #include "Frame.h"
+#include "FrameTree.h"
 #include "FrameView.h"
 #include "HTMLInputElement.h"
 #include "HTMLTextAreaElement.h"
@@ -1932,17 +1933,21 @@ void Editor::setKillRingToYankedState()
 
 #endif
 
-Editor::Visibility Editor::rangeVisibility(Range* range) const
+bool Editor::insideVisibleArea(Range* range) const
 {
+    if (!range)
+        return true;
+    
     // Right now, we only check the visibility of a range for disconnected frames. For all other
     // frames, we assume visibility.
-    if (!m_frame->isDisconnected() || !range)
-        return InsideVisibleArea;
+    Frame* frame = m_frame->isDisconnected() ? m_frame : m_frame->tree()->top(true);
+    if (!frame->isDisconnected())
+        return true;
     
-    RenderPart* renderer = m_frame->ownerRenderer();
+    RenderPart* renderer = frame->ownerRenderer();
     RenderBlock* container = renderer->containingBlock();
     if (!(container->style()->overflowX() == OHIDDEN || container->style()->overflowY() == OHIDDEN))
-        return InsideVisibleArea;
+        return true;
 
     IntRect rectInPageCoords = container->getOverflowClipRect(0, 0);
     IntRect rectInFrameCoords = IntRect(renderer->xPos() * -1, renderer->yPos() * -1,
@@ -1950,50 +1955,63 @@ Editor::Visibility Editor::rangeVisibility(Range* range) const
     IntRect resultRect = range->boundingBox();
     
     if (rectInFrameCoords.contains(resultRect))
-        return InsideVisibleArea;
-    if (resultRect.y() < rectInFrameCoords.y())
-        return BeforeVisibleArea;
-    if (resultRect.y() > rectInFrameCoords.y())
-        return AfterVisibleArea;
-
-    ASSERT(resultRect.y() == rectInFrameCoords.y());
-    if (resultRect.x() < rectInFrameCoords.x())
-        return BeforeVisibleArea;
-    return AfterVisibleArea;
+        return true;
+    return false;
 }
 
-PassRefPtr<Range> Editor::firstVisibleRange(Range* startRange, const String& target, bool forward, bool caseFlag)
+PassRefPtr<Range> Editor::firstVisibleRange(const String& target, bool caseFlag)
 {
-    if (!forward)
-        return startRange;
-
-    RefPtr<Range> resultRange = startRange;
     RefPtr<Range> searchRange(rangeOfContents(m_frame->document()));
+    RefPtr<Range> resultRange = findPlainText(searchRange.get(), target, true, caseFlag);
     ExceptionCode ec = 0;
 
-    while (rangeVisibility(resultRange.get()) == BeforeVisibleArea) {
+    while (!insideVisibleArea(resultRange.get())) {
         searchRange->setStartAfter(resultRange->endContainer(), ec);
         if (searchRange->startContainer() == searchRange->endContainer())
-            return startRange;
-        resultRange = findPlainText(searchRange.get(), target, forward, caseFlag);
+            return 0;
+        resultRange = findPlainText(searchRange.get(), target, true, caseFlag);
     }
     
     return resultRange;
 }
 
-PassRefPtr<Range> Editor::lastVisibleRange(Range* startRange, const String& target, bool forward, bool caseFlag)
+PassRefPtr<Range> Editor::lastVisibleRange(const String& target, bool caseFlag)
 {
-    if (forward)
-        return startRange;
-
-    RefPtr<Range> resultRange = startRange;
     RefPtr<Range> searchRange(rangeOfContents(m_frame->document()));
+    RefPtr<Range> resultRange = findPlainText(searchRange.get(), target, false, caseFlag);
     ExceptionCode ec = 0;
 
-    while (rangeVisibility(resultRange.get()) == AfterVisibleArea) {
+    while (!insideVisibleArea(resultRange.get())) {
         searchRange->setEndBefore(resultRange->startContainer(), ec);
         if (searchRange->startContainer() == searchRange->endContainer())
-            return startRange;
+            return 0;
+        resultRange = findPlainText(searchRange.get(), target, false, caseFlag);
+    }
+    
+    return resultRange;
+}
+
+PassRefPtr<Range> Editor::nextVisibleRange(Range* currentRange, const String& target, bool forward, bool caseFlag)
+{
+    RefPtr<Range> resultRange = currentRange;
+    RefPtr<Range> searchRange(rangeOfContents(m_frame->document()));
+    ExceptionCode ec = 0;
+    
+    while (!insideVisibleArea(resultRange.get())) {
+        if (forward)
+            searchRange->setStartAfter(resultRange->endContainer(), ec);
+        else
+            searchRange->setEndBefore(resultRange->startContainer(), ec);
+
+        // If we have made it to the beginning or the end of the document, then either there is no search result
+        // or we have to wrap around to find it.
+        if (resultRange->startContainer()->isDocumentNode()) {
+            if (forward)
+                return firstVisibleRange(target, caseFlag);
+            else
+                return lastVisibleRange(target, caseFlag);
+        }
+        
         resultRange = findPlainText(searchRange.get(), target, forward, caseFlag);
     }
     
index 28ef6b7269967f71fa765ff46b4758b983257efc..8bfc4f75c7c4292380841df5da99f9bc427cdf05 100644 (file)
@@ -255,10 +255,8 @@ public:
     PassRefPtr<Range> selectedRange();
     
     // We should make these functions private when their callers in Frame are moved over here to Editor
-    enum Visibility { BeforeVisibleArea, InsideVisibleArea, AfterVisibleArea };
-    Visibility rangeVisibility(Range*) const;
-    PassRefPtr<Range> firstVisibleRange(Range*, const String&, bool forward, bool caseFlag);
-    PassRefPtr<Range> lastVisibleRange(Range*, const String&, bool forward, bool caseFlag);
+    bool insideVisibleArea(Range*) const;
+    PassRefPtr<Range> nextVisibleRange(Range*, const String&, bool forward, bool caseFlag);
 
 private:
     Frame* m_frame;
@@ -288,6 +286,9 @@ private:
     void setIgnoreCompositionSelectionChange(bool ignore);
 
     void addToKillRing(Range*, bool prepend);
+
+    PassRefPtr<Range> firstVisibleRange(const String&, bool caseFlag);
+    PassRefPtr<Range> lastVisibleRange(const String&, bool caseFlag);
 };
 
 inline void Editor::setStartNewKillRingSequence(bool flag)
index cf4663b04a8968bd90c7c697fca949993f679ecd..1312f0b0460aa90bfa4d843d9e668e43781da6ec 100644 (file)
@@ -1541,26 +1541,20 @@ bool Frame::findString(const String& target, bool forward, bool caseFlag, bool w
         resultRange = findPlainText(searchRange.get(), target, forward, caseFlag);
     }
     
-    Editor::Visibility rangeLocation = editor()->rangeVisibility(resultRange.get());
-    if (rangeLocation == Editor::BeforeVisibleArea && forward)
-        resultRange = editor()->firstVisibleRange(resultRange.get(), target, forward, caseFlag);
+    if (!editor()->insideVisibleArea(resultRange.get())) {
+        resultRange = editor()->nextVisibleRange(resultRange.get(), target, forward, caseFlag);
+        if (!resultRange)
+            return false;
+    }
 
     // If we didn't find anything and we're wrapping, search again in the entire document (this will
     // redundantly re-search the area already searched in some cases).
-    bool disconnectedFrameShouldWrap = rangeLocation == Editor::AfterVisibleArea
-        || (rangeLocation == Editor::BeforeVisibleArea && !forward);
-    if (resultRange->collapsed(exception) && wrapFlag || disconnectedFrameShouldWrap) {
+    if (resultRange->collapsed(exception) && wrapFlag) {
         searchRange = rangeOfContents(document());
         resultRange = findPlainText(searchRange.get(), target, forward, caseFlag);
         // We used to return false here if we ended up with the same range that we started with
         // (e.g., the selection was already the only instance of this text). But we decided that
         // this should be a success case instead, so we'll just fall through in that case.
-        
-        rangeLocation = editor()->rangeVisibility(resultRange.get());
-        if (rangeLocation == Editor::BeforeVisibleArea)
-            resultRange = editor()->firstVisibleRange(resultRange.get(), target, forward, caseFlag);
-        else if (rangeLocation == Editor::AfterVisibleArea)
-            resultRange = editor()->lastVisibleRange(resultRange.get(), target, forward, caseFlag);
     }
 
     if (resultRange->collapsed(exception))
@@ -1596,19 +1590,12 @@ unsigned Frame::markAllMatchesForText(const String& target, bool caseFlag, unsig
         VisiblePosition newStart = endVisiblePosition(resultRange.get(), DOWNSTREAM);
         if (newStart == startVisiblePosition(searchRange.get(), DOWNSTREAM))
             break;
-        
-        Editor::Visibility rangeLocation = editor()->rangeVisibility(resultRange.get());
-        if (rangeLocation == Editor::BeforeVisibleArea) {
-            searchRange = rangeOfContents(document());
-            searchRange->setStartAfter(resultRange->startContainer()->shadowAncestorNode(), exception);
-            continue;
-        }
-        if (rangeLocation == Editor::AfterVisibleArea)
-            break;
 
-        ++matchCount;
-        
-        document()->addMarker(resultRange.get(), DocumentMarker::TextMatch);        
+        // Only treat the result as a match if it is visible
+        if (editor()->insideVisibleArea(resultRange.get())) {
+            ++matchCount;
+            document()->addMarker(resultRange.get(), DocumentMarker::TextMatch);
+        }
         
         // Stop looking if we hit the specified limit. A limit of 0 means no limit.
         if (limit > 0 && matchCount >= limit)