[BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNext...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Apr 2012 20:02:29 +0000 (20:02 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Apr 2012 20:02:29 +0000 (20:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=82643

Source/WebKit/blackberry:

Enhance BlackBerry::WebKit::WebPage::findNextString()

This patch adds support for toggling case sensitivity,
search wrapping, and whether or not to highlight all matches
in addition to the next found match.

I refactored and renamed the new setActiveMatchAndMarker() method
to move the active match from one range to another. This was
required because in the case of a non wrapped search we do not
want to adjust the m_activeMatch if another match is not found.

Internal Review by Andy Chen.

Patch by Mike Lattanzio <mlattanzio@rim.com> on 2012-04-03
Reviewed by Rob Buis.

* Api/WebPage.cpp:
(BlackBerry::WebKit::WebPage::findNextString):
* Api/WebPage.h:
* WebKitSupport/InPageSearchManager.cpp:
(BlackBerry::WebKit::InPageSearchManager::InPageSearchManager):
(BlackBerry::WebKit::InPageSearchManager::findNextString):
(BlackBerry::WebKit::InPageSearchManager::findAndMarkText):
(BlackBerry::WebKit::InPageSearchManager::setActiveMatchAndMarker):
(BlackBerry::WebKit::InPageSearchManager::scopeStringMatches):
* WebKitSupport/InPageSearchManager.h:
(InPageSearchManager):

Tools:

Update LayoutTestController to accomodate the new find API.
It now provides caseSensitive functionality to DRT.

Internal Review by Andy Chen.

Patch by Mike Lattanzio <mlattanzio@rim.com> on 2012-04-03
Reviewed by Rob Buis.

* DumpRenderTree/blackberry/LayoutTestControllerBlackBerry.cpp:
(LayoutTestController::findString):

LayoutTests:

Update findString-markers to also test caseSensitive.
All findString tests used to be case insensitive before.

Internal Review by Andy Chen.

Patch by Mike Lattanzio <mlattanzio@rim.com> on 2012-04-03
Reviewed by Rob Buis.

* platform/blackberry/editing/text-iterator/findString-markers-expected.txt:
* platform/blackberry/editing/text-iterator/findString-markers.html:

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

LayoutTests/ChangeLog
LayoutTests/platform/blackberry/editing/text-iterator/findString-markers-expected.txt
LayoutTests/platform/blackberry/editing/text-iterator/findString-markers.html
Source/WebKit/blackberry/Api/WebPage.cpp
Source/WebKit/blackberry/Api/WebPage.h
Source/WebKit/blackberry/ChangeLog
Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.cpp
Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.h
Tools/ChangeLog
Tools/DumpRenderTree/blackberry/LayoutTestControllerBlackBerry.cpp

index a12e4de..251572d 100644 (file)
@@ -1,3 +1,18 @@
+2012-04-03  Mike Lattanzio  <mlattanzio@rim.com>
+
+        [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNextString()
+        https://bugs.webkit.org/show_bug.cgi?id=82643
+
+        Update findString-markers to also test caseSensitive.
+        All findString tests used to be case insensitive before.
+
+        Internal Review by Andy Chen.
+
+        Reviewed by Rob Buis.
+
+        * platform/blackberry/editing/text-iterator/findString-markers-expected.txt:
+        * platform/blackberry/editing/text-iterator/findString-markers.html:
+
 2012-04-03  Ojan Vafai  <ojan@chromium.org>
 
         Update fast/regions expectations to match what's happening on
index e511e74..a64a7c8 100644 (file)
@@ -1,12 +1,9 @@
 PASS successfullyParsed is true
 
 TEST COMPLETE
-PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "a" is "a"
-PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "b" is "b"
-PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "c" is "c"
+PASS internals.markerCountForNode(e.firstChild, "textmatch") is 0
+PASS internals.markerCountForNode(e.firstChild, "textmatch") is 0
+PASS internals.markerCountForNode(e.firstChild, "textmatch") is 0
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
 PASS "d" is "d"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 3
@@ -16,36 +13,31 @@ PASS "e" is "e"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
 PASS "f" is "f"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "g" is "g"
-PASS internals.markerCountForNode(e.firstChild, "textmatch") is 2
-PASS "h" is "h"
-PASS "h" is "h"
+PASS "T" is "T"
+PASS internals.markerCountForNode(e.firstChild, "textmatch") is 0
+PASS internals.markerCountForNode(e.firstChild, "textmatch") is 0
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "i" is "i"
+PASS "x" is "x"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "j" is "j"
+PASS "y" is "y"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "k" is "k"
+PASS "z" is "z"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "l" is "l"
+PASS "." is "."
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "m" is "m"
+PASS "a" is "a"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "n" is "n"
-PASS internals.markerCountForNode(e.firstChild, "textmatch") is 4
-PASS "o" is "o"
-PASS "o" is "o"
-PASS "o" is "o"
-PASS "o" is "o"
+PASS "b" is "b"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "p" is "p"
+PASS "c" is "c"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "q" is "q"
-PASS internals.markerCountForNode(e.firstChild, "textmatch") is 2
-PASS "r" is "r"
-PASS "r" is "r"
+PASS "d" is "d"
+PASS internals.markerCountForNode(e.firstChild, "textmatch") is 3
+PASS "e" is "e"
+PASS "e" is "e"
+PASS "e" is "e"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "s" is "s"
+PASS "f" is "f"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 2
 PASS "T" is "T"
 PASS "t" is "t"
@@ -55,8 +47,6 @@ PASS "u" is "u"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
 PASS "v" is "v"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
-PASS "w" is "w"
-PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
 PASS "x" is "x"
 PASS internals.markerCountForNode(e.firstChild, "textmatch") is 1
 PASS "y" is "y"
index 6240087..c9c29e0 100644 (file)
@@ -33,44 +33,55 @@ function createPlainTextElement(id, text, parent) {
     return e;
 }
 
-function occurrences(string, substring) {
+function occurrences(string, substring, caseSensitive) {
     var n=0;
     var pos=0;
 
+    var haystack = caseSensitive ? string : string.toLowerCase();
+    var needle = caseSensitive ? substring : substring.toLowerCase();
+
     while (true) {
-        pos = string.toLowerCase().indexOf(substring.toLowerCase(),pos);
+        pos = haystack.indexOf(needle,pos);
         if (pos!=-1) {
             ++n;
-            pos+=substring.length;
+            pos+=needle.length;
         } else
             break;
     }
     return n;
 }
 
-function checkTextMatchMarker(index) {
-    if (index > 26) {
-        layoutTestController.notifyDone();
-        return;
+function checkTextMatchMarker(index, findOptions) {
+    if (index > 12) {
+        if (findOptions.length == 0) {
+            index = 0;
+            findOptions.push("CaseInsensitive");
+        } else {
+            layoutTestController.notifyDone();
+            return;
+        }
     }
-    var str = 'abcdefghijklmnopqrstuvwxyz.';
+    var str = 'ABCdefTUVxyz.';
     var char = str.charAt(index);
-    layoutTestController.findString(str.charAt(index), []);
+    var caseSensitive = findOptions.indexOf("CaseInsensitive") == -1;
+    layoutTestController.findString(char, findOptions);
     // Start the function later to allow marking process to finish.
     setTimeout(function() {
-        var count = occurrences(text, char);
+        var count = occurrences(text, char, caseSensitive);
+
         // The number of markers should match to the occurrences of the charactor.
         shouldBe('internals.markerCountForNode(e.firstChild, "textmatch")', count.toString());
         var searchStartIndex = 0;
         // Get the char from text and compare with the marker range content.
         for (var i = 0; i < count ; ++i) {
             var range = internals.markerRangeForNode(e.firstChild, "textmatch", i);
-            var pos = text.toLowerCase().indexOf(char.toLowerCase(), searchStartIndex);
+            var pos = caseSensitive ? text.indexOf(char, searchStartIndex)
+                                    : text.toLowerCase().indexOf(char.toLowerCase(), searchStartIndex);
             searchStartIndex = pos + 1;
             var expectStr = text.charAt(pos);
             shouldBe('"' + range + '"', '"' + expectStr + '"');
         }
-        checkTextMatchMarker(++index);
+        checkTextMatchMarker(index + 1, findOptions);
     }, 850);
 }
 
@@ -81,7 +92,7 @@ var text = 'The quick brown fox jumps over the lazy dog.';
 var e = createPlainTextElement(id, text, container);
 
 if (window.internals && window.layoutTestController)
-    checkTextMatchMarker(0);
+    checkTextMatchMarker(0, []);
 
 var successfullyParsed = true;
 </script>
index 5cc6d67..a7f4c5d 100644 (file)
@@ -4655,9 +4655,18 @@ void WebPage::setFocused(bool focused)
     focusController->setFocused(focused);
 }
 
-bool WebPage::findNextString(const char* text, bool forward)
-{
-    return d->m_inPageSearchManager->findNextString(String::fromUTF8(text), forward);
+bool WebPage::findNextString(const char* text, bool forward, bool caseSensitive, bool wrap, bool highlightAllMatches)
+{
+    WebCore::FindOptions findOptions = WebCore::StartInSelection;
+    if (!forward)
+        findOptions |= WebCore::Backwards;
+    if (!caseSensitive)
+        findOptions |= WebCore::CaseInsensitive;
+
+    // The WebCore::FindOptions::WrapAround boolean actually wraps the search
+    // within the current frame as opposed to the entire Document, so we have to
+    // provide our own wrapping code to wrap at the whole Document level.
+    return d->m_inPageSearchManager->findNextString(String::fromUTF8(text), findOptions, wrap, highlightAllMatches);
 }
 
 void WebPage::runLayoutTests()
index 3cd7dac..9044afa 100644 (file)
@@ -197,11 +197,9 @@ public:
 
     void runLayoutTests();
 
-     // Finds and selects the next utf8 string that is a case sensitive
-     // match in the web page. It will wrap the web page if it reaches
-     // the end. An empty string will result in no match and no selection.
-     // Returns true if the string matched and false if not.
-    bool findNextString(const char*, bool forward = true);
+    // Find the next utf8 string in the given direction.
+    // Case sensitivity, wrapping, and highlighting all matches are also toggleable.
+    bool findNextString(const char*, bool forward, bool caseSensitive, bool wrap, bool highlightAllMatches);
 
     // JavaScriptDebugger interface.
     bool enableScriptDebugger();
index ef51f9e..4abdad8 100644 (file)
@@ -1,3 +1,35 @@
+2012-04-03  Mike Lattanzio  <mlattanzio@rim.com>
+
+        [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNextString()
+        https://bugs.webkit.org/show_bug.cgi?id=82643
+
+        Enhance BlackBerry::WebKit::WebPage::findNextString()
+
+        This patch adds support for toggling case sensitivity,
+        search wrapping, and whether or not to highlight all matches
+        in addition to the next found match.
+
+        I refactored and renamed the new setActiveMatchAndMarker() method
+        to move the active match from one range to another. This was
+        required because in the case of a non wrapped search we do not
+        want to adjust the m_activeMatch if another match is not found.
+
+        Internal Review by Andy Chen.
+
+        Reviewed by Rob Buis.
+
+        * Api/WebPage.cpp:
+        (BlackBerry::WebKit::WebPage::findNextString):
+        * Api/WebPage.h:
+        * WebKitSupport/InPageSearchManager.cpp:
+        (BlackBerry::WebKit::InPageSearchManager::InPageSearchManager):
+        (BlackBerry::WebKit::InPageSearchManager::findNextString):
+        (BlackBerry::WebKit::InPageSearchManager::findAndMarkText):
+        (BlackBerry::WebKit::InPageSearchManager::setActiveMatchAndMarker):
+        (BlackBerry::WebKit::InPageSearchManager::scopeStringMatches):
+        * WebKitSupport/InPageSearchManager.h:
+        (InPageSearchManager):
+
 2012-04-02  Jacky Jiang  <zhajiang@rim.com>
 
         [BlackBerry] Adapt WebPagePrivate::webContext to the API change of r109570
index f5c12a8..ca4ec2d 100644 (file)
@@ -65,8 +65,12 @@ InPageSearchManager::InPageSearchManager(WebPagePrivate* page)
     : m_webPage(page)
     , m_activeMatch(0)
     , m_resumeScopingFromRange(0)
+    , m_activeMatchCount(0)
     , m_scopingComplete(false)
+    , m_scopingCaseInsensitive(false)
     , m_locatingActiveMatch(false)
+    , m_highlightAllMatches(false)
+    , m_activeMatchIndex(0)
 {
 }
 
@@ -75,8 +79,10 @@ InPageSearchManager::~InPageSearchManager()
     cancelPendingScopingEffort();
 }
 
-bool InPageSearchManager::findNextString(const String& text, bool forward)
+bool InPageSearchManager::findNextString(const String& text, FindOptions findOptions, bool wrap, bool highlightAllMatches)
 {
+    m_highlightAllMatches = highlightAllMatches;
+
     if (!text.length()) {
         clearTextMatches();
         cancelPendingScopingEffort();
@@ -95,12 +101,13 @@ bool InPageSearchManager::findNextString(const String& text, bool forward)
 
     RefPtr<Range> searchStartingPoint(m_activeMatch);
     bool newSearch = m_activeSearchString != text;
+    bool forward = !(findOptions & WebCore::Backwards);
     if (newSearch) { // Start a new search.
         m_activeSearchString = text;
         cancelPendingScopingEffort();
+        m_scopingCaseInsensitive = findOptions & CaseInsensitive;
         m_webPage->m_page->unmarkAllTextMatches();
-    } else { // Search same string for next occurrence.
-        setMarkerActive(m_activeMatch.get(), false /* active */);
+    } else {
         // Searching for same string should start from the end of last match.
         if (m_activeMatch) {
             if (forward)
@@ -119,19 +126,23 @@ bool InPageSearchManager::findNextString(const String& text, bool forward)
 
     Frame* currentActiveMatchFrame = selection.isNone() && m_activeMatch ? m_activeMatch->ownerDocument()->frame() : m_webPage->focusedOrMainFrame();
 
-    const FindOptions findOptions = (forward ? 0 : Backwards)
-        | CaseInsensitive
-        | StartInSelection;
-
     if (findAndMarkText(text, searchStartingPoint.get(), currentActiveMatchFrame, findOptions, newSearch))
         return true;
 
     Frame* startFrame = currentActiveMatchFrame;
     do {
-        currentActiveMatchFrame = DOMSupport::incrementFrame(currentActiveMatchFrame, forward, true /* wrapFlag */);
+        currentActiveMatchFrame = DOMSupport::incrementFrame(currentActiveMatchFrame, forward, wrap);
+
+        if (!currentActiveMatchFrame) {
+            // We should only ever have a null frame if we haven't found any
+            // matches and we're not wrapping. We have searched every frame.
+            ASSERT(!wrap);
+            return false;
+        }
+
         if (findAndMarkText(text, 0, currentActiveMatchFrame, findOptions, newSearch))
             return true;
-    } while (currentActiveMatchFrame && startFrame != currentActiveMatchFrame);
+    } while (startFrame != currentActiveMatchFrame);
 
     clearTextMatches();
 
@@ -157,13 +168,25 @@ bool InPageSearchManager::shouldSearchForText(const String& text)
 
 bool InPageSearchManager::findAndMarkText(const String& text, Range* range, Frame* frame, const FindOptions& options, bool isNewSearch)
 {
-    m_activeMatch = frame->editor()->findStringAndScrollToVisible(text, range, options);
-    if (m_activeMatch) {
-        setMarkerActive(m_activeMatch.get(), true /* active */);
-        if (isNewSearch) {
-            scopeStringMatches(text, true /* reset */);
+    if (RefPtr<Range> match = frame->editor()->findStringAndScrollToVisible(text, range, options)) {
+        // Move the highlight to the new match.
+        setActiveMatchAndMarker(match);
+
+        if (m_highlightAllMatches) {
             // FIXME: If it is a not new search, we need to calculate activeMatchIndex and notify client.
+            if (isNewSearch)
+                scopeStringMatches(text, true /* reset */);
+        } else {
+            // When only showing single matches, cancel any scoping effort and ensure
+            // only the single active match is marked.
+            cancelPendingScopingEffort();
+            m_webPage->m_page->unmarkAllTextMatches();
+            m_activeMatch->ownerDocument()->markers()->addTextMatchMarker(m_activeMatch.get(), true);
+            frame->editor()->setMarkedTextMatchesAreHighlighted(true /* highlight */);
+            m_activeMatchCount = 1;
+            m_activeMatchIndex = 1;
         }
+
         return true;
     }
     return false;
@@ -177,14 +200,19 @@ void InPageSearchManager::clearTextMatches()
     m_activeMatchIndex = 0;
 }
 
-void InPageSearchManager::setMarkerActive(Range* range, bool active)
+void InPageSearchManager::setActiveMatchAndMarker(PassRefPtr<Range> range)
 {
-    if (!range)
-        return;
-    Document* doc = m_activeMatch->ownerDocument();
-    if (!doc)
-        return;
-    doc->markers()->setMarkersActive(range, active);
+    // Clear the old marker, update our range, and highlight the new range.
+    if (m_activeMatch.get()) {
+        if (Document* doc = m_activeMatch->ownerDocument())
+            doc->markers()->setMarkersActive(m_activeMatch.get(), false);
+    }
+
+    m_activeMatch = range;
+    if (m_activeMatch.get()) {
+        if (Document* doc = m_activeMatch->ownerDocument())
+            doc->markers()->setMarkersActive(m_activeMatch.get(), true);
+    }
 }
 
 void InPageSearchManager::frameUnloaded(const Frame* frame)
@@ -240,7 +268,7 @@ void InPageSearchManager::scopeStringMatches(const String& text, bool reset, Fra
     bool timeout = false;
     double startTime = currentTime();
     do {
-        RefPtr<Range> resultRange(findPlainText(searchRange.get(), text, CaseInsensitive));
+        RefPtr<Range> resultRange(findPlainText(searchRange.get(), text, m_scopingCaseInsensitive ? CaseInsensitive : 0));
         if (resultRange->collapsed(ec)) {
             if (!resultRange->startContainer()->isInShadowTree())
                 break;
index 03af8a7..f48bb8e 100644 (file)
@@ -39,7 +39,7 @@ public:
     InPageSearchManager(WebPagePrivate*);
     ~InPageSearchManager();
 
-    bool findNextString(const String& text, bool forward);
+    bool findNextString(const String&, WebCore::FindOptions, bool wrap, bool highlightAllMatches);
     void frameUnloaded(const WebCore::Frame*);
 
 private:
@@ -47,7 +47,7 @@ private:
     friend class DeferredScopeStringMatches;
 
     void clearTextMatches();
-    void setMarkerActive(WebCore::Range*, bool);
+    void setActiveMatchAndMarker(PassRefPtr<WebCore::Range>);
     bool findAndMarkText(const String&, WebCore::Range*, WebCore::Frame*, const WebCore::FindOptions&, bool);
     bool shouldSearchForText(const String&);
     void scopeStringMatches(const String& text, bool reset, WebCore::Frame* scopingFrame = 0);
@@ -62,7 +62,9 @@ private:
     String m_activeSearchString;
     int m_activeMatchCount;
     bool m_scopingComplete;
+    bool m_scopingCaseInsensitive;
     bool m_locatingActiveMatch;
+    bool m_highlightAllMatches;
     int m_activeMatchIndex;
 };
 
index 4b61f89..e2096b2 100644 (file)
@@ -1,3 +1,18 @@
+2012-04-03  Mike Lattanzio  <mlattanzio@rim.com>
+
+        [BlackBerry] Expose CaseSensitive, Wrap, and HighlightAllMatches in WebPage::findNextString()
+        https://bugs.webkit.org/show_bug.cgi?id=82643
+
+        Update LayoutTestController to accomodate the new find API.
+        It now provides caseSensitive functionality to DRT.
+
+        Internal Review by Andy Chen.
+
+        Reviewed by Rob Buis.
+
+        * DumpRenderTree/blackberry/LayoutTestControllerBlackBerry.cpp:
+        (LayoutTestController::findString):
+
 2012-04-03  Ryosuke Niwa  <rniwa@webkit.org>
 
         Another build fix after r113067. CreateWebKitBuildDirectory step is no longer needed
index f173506..e0250ec 100644 (file)
@@ -805,13 +805,16 @@ bool LayoutTestController::findString(JSContextRef context, JSStringRef target,
     WebCore::FindOptions options = 0;
 
     JSRetainPtr<JSStringRef> lengthPropertyName(Adopt, JSStringCreateWithUTF8CString("length"));
-    JSValueRef lengthValue = JSObjectGetProperty(context, optionsArray, lengthPropertyName.get(), 0);
-    if (!JSValueIsNumber(context, lengthValue))
-        return false;
+    JSValueRef lengthValue;
+    if (optionsArray) {
+        lengthValue = JSObjectGetProperty(context, optionsArray, lengthPropertyName.get(), 0);
+        if (!JSValueIsNumber(context, lengthValue))
+            return false;
+    }
 
     WTF::String nameStr = jsStringRefToWebCoreString(target);
 
-    size_t length = static_cast<size_t>(JSValueToNumber(context, lengthValue, 0));
+    size_t length = optionsArray ? static_cast<size_t>(JSValueToNumber(context, lengthValue, 0)) : 0;
     for (size_t i = 0; i < length; ++i) {
         JSValueRef value = JSObjectGetPropertyAtIndex(context, optionsArray, i, 0);
         if (!JSValueIsString(context, value))
@@ -832,7 +835,10 @@ bool LayoutTestController::findString(JSContextRef context, JSStringRef target,
         else if (JSStringIsEqualToUTF8CString(optionName.get(), "StartInSelection"))
             options |= WebCore::StartInSelection;
     }
-    return BlackBerry::WebKit::DumpRenderTree::currentInstance()->page()->findNextString(nameStr.utf8().data(), !(options | WebCore::Backwards));
+
+    // Our layout tests assume find will wrap and highlight all matches.
+    return BlackBerry::WebKit::DumpRenderTree::currentInstance()->page()->findNextString(nameStr.utf8().data(),
+        !(options & WebCore::Backwards), !(options & WebCore::CaseInsensitive), true /* wrap */, true /* highlightAllMatches */);
 }
 
 void LayoutTestController::deleteLocalStorageForOrigin(JSStringRef URL)