Changeset 143391 in webkit

Timestamp:

Feb 19, 2013 3:15:42 PM (11 years ago)

Author:

nghanavatian@rim.com

Message:

[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):

Location:

trunk/Source/WebKit/blackberry

Files:

• ChangeLog (modified) (1 diff)
• WebKitSupport/DOMSupport.cpp (modified) (3 diffs)
• WebKitSupport/DOMSupport.h (modified) (2 diffs)
• WebKitSupport/InputHandler.cpp (modified) (3 diffs)
• WebKitSupport/SpellingHandler.cpp (modified) (6 diffs)
• WebKitSupport/SpellingHandler.h (modified) (1 diff)

trunk/Source/WebKit/blackberry/ChangeLog

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

trunk/Source/WebKit/blackberry/WebKitSupport/DOMSupport.cpp

@@ -509 +509 @@
 }
 
+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)
@@ -524 +564 @@
 PassRefPtr<Range> trimWhitespaceFromRange(VisiblePosition startPosition, VisiblePosition endPosition)
 {
-    if (isEmptyRangeOrAllSpaces(startPosition, endPosition))
+    if (startPosition == endPosition || isRangeTextAllWhitespace(startPosition, endPosition))
         return 0;
 
@@ -536 +576 @@
 }
 
-bool isEmptyRangeOrAllSpaces(VisiblePosition startPosition, VisiblePosition endPosition)
-{
-    if (startPosition == endPosition)
-        return true;
-
+bool isRangeTextAllWhitespace(VisiblePosition startPosition, VisiblePosition endPosition)
+{
     while (isWhitespace(startPosition.characterAfter())) {
         startPosition = startPosition.next();

trunk/Source/WebKit/blackberry/WebKitSupport/DOMSupport.h

@@ -91 +91 @@
 
 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);
@@ -96 +98 @@
 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*);

trunk/Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp

@@ -613 +613 @@
     }
 
-    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);
@@ -620 +620 @@
         m_request->didCancel();
         if (m_currentFocusElement->document() && m_currentFocusElement->document()->frame() && m_currentFocusElement->document()->frame()->selection()) {
+            VisiblePosition caretPosition = m_currentFocusElement->document()->frame()->selection()->start();
             // 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);
+            // 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);
         }
@@ -1093 +1096 @@
 
     // 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

trunk/Source/WebKit/blackberry/WebKitSupport/SpellingHandler.cpp

@@ -42 +42 @@
 }
 
-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();
@@ -51 +49 @@
         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())
@@ -62 +62 @@
 
     // 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;
         }
@@ -71 +71 @@
 
     // 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;
@@ -87 +87 @@
         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.");
@@ -105 +113 @@
 
     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;
+    SpellingLog(Platform::LogLevelInfo, "SpellingHandler::handleOversizedRange");
 
-    // 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;
+    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;
+    }
 
-        // 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.
+    // 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;
     }
-    // This will return an range with no string, but allows processing to continue
-    return makeRange(startPosition, endPosition);
+
+    m_cachedEndPosition = m_endPosition;
+    m_endPosition = endOfWord(nextWord);
 }
 

trunk/Source/WebKit/blackberry/WebKitSupport/SpellingHandler.h

@@ -31 +31 @@
     ~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;