[BlackBerry] Appropriately handle word wrapping in SpellingHandler
authornghanavatian@rim.com <nghanavatian@rim.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2013 23:15:42 +0000 (23:15 +0000)
committernghanavatian@rim.com <nghanavatian@rim.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2013 23:15:42 +0000 (23:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110253

Reviewed by Rob Buis.

PR286001
Since we traverse through text by visual lines instead of blocks, word wrapping causes some
bad behavior. Changing the way we traverse text to jump by words instead of lines. This will
mean it takes longer to finish spellchecking, but the removal of any loops allows webkit
processing to continue. This gives priority to user actions while still completing a large
paragraph in a reasonable amount of time.

Internally reviewed by Mike Fenton

* WebKitSupport/InputHandler.cpp:
(BlackBerry::WebKit::InputHandler::requestCheckingOfString):
* WebKitSupport/SpellingHandler.cpp:
(BlackBerry::WebKit::SpellingHandler::createSpellCheckRequest):
(BlackBerry::WebKit::SpellingHandler::parseBlockForSpellChecking):
(BlackBerry::WebKit::SpellingHandler::getRangeForSpellCheckWithFineGranularity):
(BlackBerry::WebKit::SpellingHandler::startOfNextWord):
(WebKit):
(BlackBerry::WebKit::SpellingHandler::incrementByWord):
(BlackBerry::WebKit::SpellingHandler::doesWordWrap):
* WebKitSupport/SpellingHandler.h:
(SpellingHandler):

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

Source/WebKit/blackberry/ChangeLog
Source/WebKit/blackberry/WebKitSupport/DOMSupport.cpp
Source/WebKit/blackberry/WebKitSupport/DOMSupport.h
Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp
Source/WebKit/blackberry/WebKitSupport/SpellingHandler.cpp
Source/WebKit/blackberry/WebKitSupport/SpellingHandler.h

index 8d7948b22818c43c61360a040ca4ad0a12c0787f..f28fc26b6b219a260eb1f7e9f76058b886fc63fc 100644 (file)
@@ -1,3 +1,32 @@
+2013-02-19  Nima Ghanavatian  <nghanavatian@rim.com>
+
+        [BlackBerry] Appropriately handle word wrapping in SpellingHandler
+        https://bugs.webkit.org/show_bug.cgi?id=110253
+
+        Reviewed by Rob Buis.
+
+        PR286001
+        Since we traverse through text by visual lines instead of blocks, word wrapping causes some
+        bad behavior. Changing the way we traverse text to jump by words instead of lines. This will
+        mean it takes longer to finish spellchecking, but the removal of any loops allows webkit
+        processing to continue. This gives priority to user actions while still completing a large
+        paragraph in a reasonable amount of time.
+
+        Internally reviewed by Mike Fenton
+
+        * WebKitSupport/InputHandler.cpp:
+        (BlackBerry::WebKit::InputHandler::requestCheckingOfString):
+        * WebKitSupport/SpellingHandler.cpp:
+        (BlackBerry::WebKit::SpellingHandler::createSpellCheckRequest):
+        (BlackBerry::WebKit::SpellingHandler::parseBlockForSpellChecking):
+        (BlackBerry::WebKit::SpellingHandler::getRangeForSpellCheckWithFineGranularity):
+        (BlackBerry::WebKit::SpellingHandler::startOfNextWord):
+        (WebKit):
+        (BlackBerry::WebKit::SpellingHandler::incrementByWord):
+        (BlackBerry::WebKit::SpellingHandler::doesWordWrap):
+        * WebKitSupport/SpellingHandler.h:
+        (SpellingHandler):
+
 2013-02-18  Simon Fraser  <simon.fraser@apple.com>
 
         Clean up the boolean argument to visibleContentRect
index c9864a79b68721db8d4b7fcca3246d7982a3e324..3510d889e98d4f3b1aeb98372280ec10674ce405 100644 (file)
@@ -508,6 +508,46 @@ VisibleSelection visibleSelectionForClosestActualWordStart(const VisibleSelectio
     return selection;
 }
 
+int offsetFromStartOfBlock(const VisiblePosition offset)
+{
+    RefPtr<Range> range = makeRange(startOfBlock(offset), offset);
+    if (!range)
+        return -1;
+
+    return range->text().latin1().length();
+}
+
+VisibleSelection visibleSelectionForFocusedBlock(Element* element)
+{
+    int textLength = inputElementText(element).length();
+
+    if (DOMSupport::toTextControlElement(element)) {
+        RenderTextControl* textRender = toRenderTextControl(element->renderer());
+        if (!textRender)
+            return VisibleSelection();
+
+        VisiblePosition startPosition = textRender->visiblePositionForIndex(0);
+        VisiblePosition endPosition;
+
+        if (textLength)
+            endPosition = textRender->visiblePositionForIndex(textLength);
+        else
+            endPosition = startPosition;
+        return VisibleSelection(startPosition, endPosition);
+    }
+
+    // Must be content editable, generate the range.
+    RefPtr<Range> selectionRange = TextIterator::rangeFromLocationAndLength(element, 0, textLength);
+
+    if (!selectionRange)
+        return VisibleSelection();
+
+    if (!textLength)
+        return VisibleSelection(selectionRange->startPosition(), DOWNSTREAM);
+
+    return VisibleSelection(selectionRange->startPosition(), selectionRange->endPosition());
+}
+
 // This function is copied from WebCore/page/Page.cpp.
 Frame* incrementFrame(Frame* curr, bool forward, bool wrapFlag)
 {
@@ -523,7 +563,7 @@ PassRefPtr<Range> trimWhitespaceFromRange(PassRefPtr<Range> range)
 
 PassRefPtr<Range> trimWhitespaceFromRange(VisiblePosition startPosition, VisiblePosition endPosition)
 {
-    if (isEmptyRangeOrAllSpaces(startPosition, endPosition))
+    if (startPosition == endPosition || isRangeTextAllWhitespace(startPosition, endPosition))
         return 0;
 
     while (isWhitespace(startPosition.characterAfter()))
@@ -535,11 +575,8 @@ PassRefPtr<Range> trimWhitespaceFromRange(VisiblePosition startPosition, Visible
     return makeRange(startPosition, endPosition);
 }
 
-bool isEmptyRangeOrAllSpaces(VisiblePosition startPosition, VisiblePosition endPosition)
+bool isRangeTextAllWhitespace(VisiblePosition startPosition, VisiblePosition endPosition)
 {
-    if (startPosition == endPosition)
-        return true;
-
     while (isWhitespace(startPosition.characterAfter())) {
         startPosition = startPosition.next();
 
index 1e88ab50b8c813486f2ea6095f8c3b91a7d0f264..c8d7c08a86853c7b277b94d261554a45adff8963 100644 (file)
@@ -90,12 +90,14 @@ WebCore::IntPoint convertPointToFrame(const WebCore::Frame* sourceFrame, const W
 static const WebCore::IntPoint InvalidPoint = WebCore::IntPoint(-1, -1);
 
 WebCore::VisibleSelection visibleSelectionForClosestActualWordStart(const WebCore::VisibleSelection&);
+WebCore::VisibleSelection visibleSelectionForFocusedBlock(WebCore::Element*);
+int offsetFromStartOfBlock(const WebCore::VisiblePosition offset);
 
 WebCore::Frame* incrementFrame(WebCore::Frame* curr, bool forward, bool wrapFlag);
 
 PassRefPtr<WebCore::Range> trimWhitespaceFromRange(PassRefPtr<WebCore::Range>);
 PassRefPtr<WebCore::Range> trimWhitespaceFromRange(WebCore::VisiblePosition startPosition, WebCore::VisiblePosition endPosition);
-bool isEmptyRangeOrAllSpaces(WebCore::VisiblePosition, WebCore::VisiblePosition);
+bool isRangeTextAllWhitespace(WebCore::VisiblePosition startPosition, WebCore::VisiblePosition endPosition);
 
 bool isFixedPositionOrHasFixedPositionAncestor(WebCore::RenderObject*);
 
index ec96fda814d27153226d0a49c09fa57c925e848f..a52097a54a331d245bba7ab6e4288eea20dec48c 100644 (file)
@@ -612,17 +612,20 @@ void InputHandler::requestCheckingOfString(PassRefPtr<WebCore::TextCheckingReque
         return;
     }
 
-    if (requestLength > MaxSpellCheckingStringLength) {
+    if (requestLength >= MaxSpellCheckingStringLength) {
         // Batch requests which are generally created by us on focus, should not exceed this limit. Check that this is in fact of Incremental type.
         ASSERT(textCheckingRequest->processType() == TextCheckingProcessIncremental);
 
         // Cancel this request and send it off in newly created chunks.
         m_request->didCancel();
         if (m_currentFocusElement->document() && m_currentFocusElement->document()->frame() && m_currentFocusElement->document()->frame()->selection()) {
-            // Convert from position back to selection so we can expand the range to include the previous line. This should handle cases when the user hits
-            // enter to finish composing a word and create a new line.
             VisiblePosition caretPosition = m_currentFocusElement->document()->frame()->selection()->start();
-            VisibleSelection visibleSelection = VisibleSelection(previousLinePosition(caretPosition, caretPosition.lineDirectionPointForBlockDirectionNavigation()), caretPosition);
+            // Convert from position back to selection so we can expand the range to include the previous line. This should handle cases when the user hits
+            // enter to finish composing a word and create a new line. Account for word wrapping by jumping to the start of the previous line, then moving
+            // to the start of any word which might be there.
+            VisibleSelection visibleSelection = VisibleSelection(
+                startOfWord(startOfLine(previousLinePosition(caretPosition, caretPosition.lineDirectionPointForBlockDirectionNavigation()))),
+                endOfWord(endOfLine(caretPosition)));
             m_spellingHandler->spellCheckTextBlock(visibleSelection, TextCheckingProcessIncremental);
         }
         return;
@@ -1092,8 +1095,8 @@ void InputHandler::setElementFocused(Element* element)
         return;
 
     // Spellcheck the field in its entirety.
-    VisibleSelection focusedBlock = DOMSupport::visibleSelectionForInputElement(element);
-    m_spellingHandler->spellCheckTextBlock(focusedBlock, TextCheckingProcessBatch);
+    const VisibleSelection visibleSelection = DOMSupport::visibleSelectionForFocusedBlock(element);
+    m_spellingHandler->spellCheckTextBlock(visibleSelection, TextCheckingProcessBatch);
 
 #ifdef ENABLE_SPELLING_LOG
     SpellingLog(Platform::LogLevelInfo, "InputHandler::setElementFocused Spellchecking the field increased the total time to focus to %f seconds.", timer.elapsed());
index 542a54b4a9f02c03c9944e5d4f73df4d832ac2b7..9dd59a9a7311b0337b3008590b0ffc91fd686448 100644 (file)
@@ -41,18 +41,18 @@ SpellingHandler::~SpellingHandler()
 {
 }
 
-void SpellingHandler::spellCheckTextBlock(WebCore::VisibleSelection& visibleSelection, WebCore::TextCheckingProcessType textCheckingProcessType)
+void SpellingHandler::spellCheckTextBlock(const WebCore::VisibleSelection& visibleSelection, WebCore::TextCheckingProcessType textCheckingProcessType)
 {
-    SpellingLog(Platform::LogLevelInfo, "SpellingHandler::spellCheckTextBlock");
-
     // Check if this request can be sent off in one message, or if it needs to be broken down.
     RefPtr<Range> rangeForSpellChecking = visibleSelection.toNormalizedRange();
     if (!rangeForSpellChecking || !rangeForSpellChecking->text() || !rangeForSpellChecking->text().length())
         return;
 
+    m_textCheckingProcessType = textCheckingProcessType;
+
     // Spellcheck Batch requests are used when focusing an element. During this time, we might have a lingering request
     // from a previously focused element.
-    if (textCheckingProcessType == TextCheckingProcessBatch) {
+    if (m_textCheckingProcessType == TextCheckingProcessBatch) {
         // If a previous request is being processed, stop it before continueing.
         if (m_timer.isActive())
             m_timer.stop();
@@ -61,106 +61,112 @@ void SpellingHandler::spellCheckTextBlock(WebCore::VisibleSelection& visibleSele
     m_isSpellCheckActive = true;
 
     // If we have a batch request, try to send off the entire block.
-    if (textCheckingProcessType == TextCheckingProcessBatch) {
+    if (m_textCheckingProcessType == TextCheckingProcessBatch) {
         // If total block text is under the limited amount, send the entire chunk.
         if (rangeForSpellChecking->text().length() < MaxSpellCheckingStringLength) {
-            createSpellCheckRequest(rangeForSpellChecking, TextCheckingProcessBatch);
+            createSpellCheckRequest(rangeForSpellChecking);
             return;
         }
     }
 
     // Since we couldn't check the entire block at once, set up starting and ending markers to fire incrementally.
-    m_startOfCurrentLine = startOfLine(visibleSelection.visibleStart());
-    m_endOfCurrentLine = endOfLine(m_startOfCurrentLine);
-    m_textCheckingProcessType = textCheckingProcessType;
+    // Find the start and end of the region we're intending on checking
+    m_startPosition = visibleSelection.visibleStart();
+    m_endPosition = endOfWord(m_startPosition);
+    m_endOfRange = visibleSelection.visibleEnd();
+    m_cachedEndPosition = m_endOfRange;
 
-    // Find the end of the region we're intending on checking.
-    m_endOfRange = endOfLine(visibleSelection.visibleEnd());
     m_timer.startOneShot(0);
 }
 
-void SpellingHandler::createSpellCheckRequest(PassRefPtr<WebCore::Range> rangeForSpellCheckingPtr, WebCore::TextCheckingProcessType textCheckingProcessType)
+void SpellingHandler::createSpellCheckRequest(const PassRefPtr<WebCore::Range> rangeForSpellCheckingPtr)
 {
     RefPtr<WebCore::Range> rangeForSpellChecking = rangeForSpellCheckingPtr;
     rangeForSpellChecking = DOMSupport::trimWhitespaceFromRange(rangeForSpellChecking);
     if (!rangeForSpellChecking)
         return;
 
-    SpellingLog(Platform::LogLevelInfo, "SpellingHandler::createSpellCheckRequest Substring text is '%s', of size %d"
-        , rangeForSpellChecking->text().latin1().data()
-        , rangeForSpellChecking->text().length());
-
-    if (rangeForSpellChecking->text().length() >= MinSpellCheckingStringLength)
-        m_inputHandler->callRequestCheckingFor(SpellCheckRequest::create(TextCheckingTypeSpelling, textCheckingProcessType, rangeForSpellChecking, rangeForSpellChecking));
+    if (rangeForSpellChecking->text().length() >= MinSpellCheckingStringLength) {
+        SpellingLog(Platform::LogLevelInfo, "SpellingHandler::createSpellCheckRequest Substring text is '%s', of size %d"
+            , rangeForSpellChecking->text().latin1().data()
+            , rangeForSpellChecking->text().length());
+        m_inputHandler->callRequestCheckingFor(SpellCheckRequest::create(TextCheckingTypeSpelling, m_textCheckingProcessType, rangeForSpellChecking, rangeForSpellChecking));
+    }
 }
 
 void SpellingHandler::parseBlockForSpellChecking(WebCore::Timer<SpellingHandler>*)
 {
-    // Create a selection with the start and end points of the line, and convert to Range to create a SpellCheckRequest.
-    RefPtr<Range> rangeForSpellChecking = makeRange(m_startOfCurrentLine, m_endOfCurrentLine);
+    SpellingLog(Platform::LogLevelInfo, "SpellingHandler::parseBlockForSpellChecking m_startPosition = %d, m_endPosition = %d, m_cachedEndPosition = %d, m_endOfRange = %d"
+        , DOMSupport::offsetFromStartOfBlock(m_startPosition)
+        , DOMSupport::offsetFromStartOfBlock(m_endPosition)
+        , DOMSupport::offsetFromStartOfBlock(m_cachedEndPosition)
+        , DOMSupport::offsetFromStartOfBlock(m_endOfRange));
+
+    if (m_startPosition == m_endOfRange)
+        return;
+
+    RefPtr<Range> rangeForSpellChecking = makeRange(m_startPosition, m_endPosition);
     if (!rangeForSpellChecking) {
         SpellingLog(Platform::LogLevelInfo, "SpellingHandler::parseBlockForSpellChecking Failed to set text range for spellchecking.");
         return;
     }
 
     if (rangeForSpellChecking->text().length() < MaxSpellCheckingStringLength) {
-        if (m_endOfCurrentLine == m_endOfRange) {
-            createSpellCheckRequest(rangeForSpellChecking, m_textCheckingProcessType);
+        if (m_endPosition == m_endOfRange || m_cachedEndPosition == m_endPosition) {
+            createSpellCheckRequest(rangeForSpellChecking);
             m_isSpellCheckActive = false;
             return;
         }
-        m_endOfCurrentLine = endOfLine(nextLinePosition(m_endOfCurrentLine, m_endOfCurrentLine.lineDirectionPointForBlockDirectionNavigation()));
+
+        incrementSentinels(false /* shouldIncrementStartPosition */);
         m_timer.startOneShot(s_timeout);
         return;
     }
 
-    // Iterate through words from the start of the line to the end.
-    rangeForSpellChecking = getRangeForSpellCheckWithFineGranularity(m_startOfCurrentLine, m_endOfCurrentLine);
-    if (!rangeForSpellChecking)
-        return;
+    // Create a spellcheck request with the substring if we have a range that is of size less than MaxSpellCheckingStringLength
+    if (rangeForSpellChecking = handleOversizedRange())
+        createSpellCheckRequest(rangeForSpellChecking);
 
-    m_startOfCurrentLine = VisiblePosition(rangeForSpellChecking->endPosition());
-    m_endOfCurrentLine = endOfLine(m_startOfCurrentLine);
-
-    // Call spellcheck with substring.
-    createSpellCheckRequest(rangeForSpellChecking, m_textCheckingProcessType);
     if (isSpellCheckActive())
         m_timer.startOneShot(s_timeout);
 }
 
-PassRefPtr<Range> SpellingHandler::getRangeForSpellCheckWithFineGranularity(WebCore::VisiblePosition startPosition, WebCore::VisiblePosition endPosition)
+PassRefPtr<Range> SpellingHandler::handleOversizedRange()
 {
-    SpellingLog(Platform::LogLevelWarn, "SpellingHandler::getRangeForSpellCheckWithFineGranularity");
-    VisiblePosition endOfCurrentWord = endOfWord(startPosition);
-    RefPtr<Range> currentRange;
-
-    // Keep iterating until one of our cases is hit, or we've incremented the starting position right to the end.
-    while (startPosition != endPosition) {
-        currentRange = makeRange(startPosition, endOfCurrentWord);
-        if (!currentRange)
-            return 0;
-
-        // Check the text length within this range.
-        if (currentRange->text().length() >= MaxSpellCheckingStringLength) {
-            // If this is not the first word, return a Range with end boundary set to the previous word.
-            if (startOfWord(endOfCurrentWord, LeftWordIfOnBoundary) != startPosition && !DOMSupport::isEmptyRangeOrAllSpaces(startPosition, endOfCurrentWord)) {
-                // When a series of whitespace follows a word, previousWordPosition will jump passed all of them, and using LeftWordIfOnBoundary brings us to
-                // our starting position. Check for this case and use RightWordIfOnBoundary to jump back over the word.
-                VisiblePosition endOfPreviousWord = endOfWord(previousWordPosition(endOfCurrentWord), LeftWordIfOnBoundary);
-                if (startPosition == endOfPreviousWord)
-                    return makeRange(startPosition, endOfWord(previousWordPosition(endOfCurrentWord), RightWordIfOnBoundary));
-                return makeRange(startPosition, endOfPreviousWord);
-            }
-            // Our first word has gone over the character limit. Increment the starting position past an uncheckable word.
-            startPosition = endOfCurrentWord;
-            endOfCurrentWord = endOfWord(nextWordPosition(endOfCurrentWord));
-        } else if (endOfCurrentWord == endPosition)
-            return makeRange(startPosition, endPosition); // Return the last segment if the end of our word lies at the end of the range.
-        else
-            endOfCurrentWord = endOfWord(nextWordPosition(endOfCurrentWord)); // Increment the current word.
+    SpellingLog(Platform::LogLevelInfo, "SpellingHandler::handleOversizedRange");
+
+    if (m_startPosition == m_cachedEndPosition || m_startPosition == startOfWord(m_endPosition, LeftWordIfOnBoundary)) {
+        // Our first word has gone over the character limit. Increment the starting position past an uncheckable word.
+        incrementSentinels(true /* shouldIncrementStartPosition */);
+        return 0;
     }
-    // This will return an range with no string, but allows processing to continue
-    return makeRange(startPosition, endPosition);
+
+    // If this is not the first word, return a Range with end boundary set to the previous word.
+    RefPtr<Range> rangeToStartOfOversizedWord = makeRange(m_startPosition, m_cachedEndPosition);
+    // We've created the range using the cached end position. Now increment the sentinals forward.
+    // FIXME Incrementing the start/end positions outside of incrementSentinels
+    m_startPosition = m_cachedEndPosition;
+    m_endPosition = endOfWord(m_startPosition);
+    return rangeToStartOfOversizedWord;
+}
+
+void SpellingHandler::incrementSentinels(bool shouldIncrementStartPosition)
+{
+    SpellingLog(Platform::LogLevelInfo, "SpellingHandler::incrementSentinels shouldIncrementStartPosition %s", shouldIncrementStartPosition ? "true" : "false");
+
+    if (shouldIncrementStartPosition)
+        m_startPosition = m_endPosition;
+
+    VisiblePosition nextWord = nextWordPosition(m_endPosition);
+    VisiblePosition startOfNextWord = startOfWord(nextWord, LeftWordIfOnBoundary);
+    if (DOMSupport::isRangeTextAllWhitespace(m_endPosition, startOfNextWord)) {
+        m_cachedEndPosition = startOfNextWord;
+        m_endPosition = endOfWord(startOfNextWord);
+        return;
+    }
+
+    m_cachedEndPosition = m_endPosition;
+    m_endPosition = endOfWord(nextWord);
 }
 
 } // WebKit
index 22a905255e03b7c538cdfc47542d4c6fd8903c09..1e59231da9c6adc07b4646e214b61a336ac5263e 100644 (file)
@@ -30,21 +30,23 @@ public:
     SpellingHandler(InputHandler*);
     ~SpellingHandler();
 
-    void spellCheckTextBlock(WebCore::VisibleSelection&, WebCore::TextCheckingProcessType);
+    void spellCheckTextBlock(const WebCore::VisibleSelection&, const WebCore::TextCheckingProcessType);
     bool isSpellCheckActive() { return m_isSpellCheckActive; }
     void setSpellCheckActive(bool active) { m_isSpellCheckActive = active; }
 
 private:
-    void createSpellCheckRequest(PassRefPtr<WebCore::Range> rangeForSpellCheckingPtr, WebCore::TextCheckingProcessType);
+    void createSpellCheckRequest(PassRefPtr<WebCore::Range> rangeForSpellCheckingPtr);
     void parseBlockForSpellChecking(WebCore::Timer<SpellingHandler>*);
-    PassRefPtr<WebCore::Range> getRangeForSpellCheckWithFineGranularity(WebCore::VisiblePosition startPosition, WebCore::VisiblePosition endPosition);
+    void incrementSentinels(bool shouldIncrementStartPosition);
+    PassRefPtr<WebCore::Range> handleOversizedRange();
 
     InputHandler* m_inputHandler;
 
     WebCore::Timer<SpellingHandler> m_timer;
-    WebCore::VisiblePosition m_startOfCurrentLine;
-    WebCore::VisiblePosition m_endOfCurrentLine;
-    WebCore::VisibleSelection m_endOfRange;
+    WebCore::VisiblePosition m_startPosition;
+    WebCore::VisiblePosition m_endPosition;
+    WebCore::VisiblePosition m_cachedEndPosition;
+    WebCore::VisiblePosition m_endOfRange;
     WebCore::TextCheckingProcessType m_textCheckingProcessType;
     bool m_isSpellCheckActive;
 };