Redo spellchecking of a field if the layout has changed
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Apr 2013 23:52:44 +0000 (23:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Apr 2013 23:52:44 +0000 (23:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=114700

Patch by Nima Ghanavatian <nghanavatian@blackberry.com> on 2013-04-16
Reviewed by Rob Buis.
Internally reviewed by Mike Fenton.

PR258637
If we insert a child node during spellchecking, the current request along
with the requests in queue become stale. The offsets were calculated when
they were created are no longer valid. We clear the queue by setting sequence
id to -1 and trigger spell checking again. We only trigger re-checking
if the layout change occurred during processing of a request. This is
maintained with the m_request pointer as it should be cleared after use.

* Api/WebPage.cpp:
(BlackBerry::WebKit::WebPagePrivate::layoutFinished):
* WebCoreSupport/EditorClientBlackBerry.cpp:
(WebCore::EditorClientBlackBerry::requestCheckingOfString):
* WebKitSupport/InputHandler.cpp:
(BlackBerry::WebKit::InputHandler::requestCheckingOfString):
(BlackBerry::WebKit::InputHandler::spellCheckingRequestCancelled):
(BlackBerry::WebKit::InputHandler::spellCheckingRequestProcessed):
(BlackBerry::WebKit::InputHandler::setElementFocused):
(BlackBerry::WebKit::InputHandler::spellCheckTextBlock):
(WebKit):
(BlackBerry::WebKit::InputHandler::stopPendingSpellCheckRequests):
* WebKitSupport/InputHandler.h:
(InputHandler):

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

Source/WebKit/blackberry/Api/WebPage.cpp
Source/WebKit/blackberry/ChangeLog
Source/WebKit/blackberry/WebCoreSupport/EditorClientBlackBerry.cpp
Source/WebKit/blackberry/WebKitSupport/InputHandler.cpp
Source/WebKit/blackberry/WebKitSupport/InputHandler.h

index cdc6280..d100300 100644 (file)
@@ -1568,6 +1568,10 @@ void WebPagePrivate::overflowExceedsContentsSize()
 
 void WebPagePrivate::layoutFinished()
 {
+    // If a layout change has occurred, we need to invalidate any current spellcheck requests and trigger a new run.
+    m_inputHandler->stopPendingSpellCheckRequests();
+    m_inputHandler->spellCheckTextBlock();
+
     if (!m_contentsSizeChanged && !m_overflowExceedsContentsSize)
         return;
 
index e1dfe55..f88213c 100644 (file)
@@ -1,3 +1,34 @@
+2013-04-16  Nima Ghanavatian  <nghanavatian@blackberry.com>
+
+        Redo spellchecking of a field if the layout has changed
+        https://bugs.webkit.org/show_bug.cgi?id=114700
+
+        Reviewed by Rob Buis.
+        Internally reviewed by Mike Fenton.
+
+        PR258637
+        If we insert a child node during spellchecking, the current request along
+        with the requests in queue become stale. The offsets were calculated when
+        they were created are no longer valid. We clear the queue by setting sequence
+        id to -1 and trigger spell checking again. We only trigger re-checking
+        if the layout change occurred during processing of a request. This is
+        maintained with the m_request pointer as it should be cleared after use.
+
+        * Api/WebPage.cpp:
+        (BlackBerry::WebKit::WebPagePrivate::layoutFinished):
+        * WebCoreSupport/EditorClientBlackBerry.cpp:
+        (WebCore::EditorClientBlackBerry::requestCheckingOfString):
+        * WebKitSupport/InputHandler.cpp:
+        (BlackBerry::WebKit::InputHandler::requestCheckingOfString):
+        (BlackBerry::WebKit::InputHandler::spellCheckingRequestCancelled):
+        (BlackBerry::WebKit::InputHandler::spellCheckingRequestProcessed):
+        (BlackBerry::WebKit::InputHandler::setElementFocused):
+        (BlackBerry::WebKit::InputHandler::spellCheckTextBlock):
+        (WebKit):
+        (BlackBerry::WebKit::InputHandler::stopPendingSpellCheckRequests):
+        * WebKitSupport/InputHandler.h:
+        (InputHandler):
+
 2013-04-16  Jacky Jiang  <zhajiang@blackberry.com>
 
         [BlackBerry] Viewport not rendered correctly
index 9b423e2..cf0cccb 100644 (file)
@@ -34,6 +34,7 @@
 #include "PlatformKeyboardEvent.h"
 #include "SelectionHandler.h"
 #include "Settings.h"
+#include "SpellChecker.h"
 #include "WebPage_p.h"
 #include "WindowsKeyboardCodes.h"
 
@@ -580,7 +581,8 @@ void EditorClientBlackBerry::checkGrammarOfString(const UChar*, int, WTF::Vector
 
 void EditorClientBlackBerry::requestCheckingOfString(PassRefPtr<TextCheckingRequest> testCheckingRequest)
 {
-    m_webPagePrivate->m_inputHandler->requestCheckingOfString(textCheckingRequest);
+    RefPtr<SpellCheckRequest> spellCheckRequest = static_cast<SpellCheckRequest*>(textCheckingRequest.get());
+    m_webPagePrivate->m_inputHandler->requestCheckingOfString(spellCheckRequest);
 }
 
 void EditorClientBlackBerry::checkTextOfParagraph(const UChar*, int, TextCheckingTypeMask, Vector<TextCheckingResult>&)
index fb3fa5e..4bc37f3 100644 (file)
@@ -599,31 +599,29 @@ void InputHandler::callRequestCheckingFor(PassRefPtr<WebCore::SpellCheckRequest>
         spellChecker->requestCheckingFor(spellCheckRequest);
 }
 
-void InputHandler::requestCheckingOfString(PassRefPtr<WebCore::TextCheckingRequest> textCheckingRequest)
+void InputHandler::requestCheckingOfString(PassRefPtr<WebCore::SpellCheckRequest> spellCheckRequest)
 {
-    m_request = textCheckingRequest;
+    SpellingLog(Platform::LogLevelInfo, "InputHandler::requestCheckingOfString '%s'", spellCheckRequest->data().text().latin1().data());
 
-    InputLog(Platform::LogLevelInfo, "InputHandler::requestCheckingOfString '%s'", m_request->data().text().latin1().data());
-
-    if (!m_request) {
+    if (!spellCheckRequest) {
         SpellingLog(Platform::LogLevelWarn, "InputHandler::requestCheckingOfString did not receive a valid request.");
         return;
     }
 
-    unsigned requestLength = m_request->data().text().length();
+    unsigned requestLength = spellCheckRequest->data().text().length();
 
     // Check if the field should be spellchecked.
     if (!isActiveTextEdit() || !shouldSpellCheckElement(m_currentFocusElement.get()) || requestLength < 2) {
-        m_request->didCancel();
+        spellCheckRequest->didCancel();
         return;
     }
 
     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);
+        ASSERT(spellCheckRequest->data().processType() == TextCheckingProcessIncremental);
 
         // Cancel this request and send it off in newly created chunks.
-        m_request->didCancel();
+        spellCheckRequest->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
@@ -640,15 +638,15 @@ void InputHandler::requestCheckingOfString(PassRefPtr<WebCore::TextCheckingReque
     wchar_t* checkingString = (wchar_t*)malloc(sizeof(wchar_t) * (requestLength + 1));
     if (!checkingString) {
         Platform::logAlways(Platform::LogLevelCritical, "InputHandler::requestCheckingOfString Cannot allocate memory for string.");
-        m_request->didCancel();
+        spellCheckRequest->didCancel();
         return;
     }
 
     int paragraphLength = 0;
-    if (!convertStringToWchar(m_request->data().text(), checkingString, requestLength + 1, &paragraphLength)) {
+    if (!convertStringToWchar(spellCheckRequest->data().text(), checkingString, requestLength + 1, &paragraphLength)) {
         Platform::logAlways(Platform::LogLevelCritical, "InputHandler::requestCheckingOfString Failed to convert String to wchar type.");
         free(checkingString);
-        m_request->didCancel();
+        spellCheckRequest->didCancel();
         return;
     }
 
@@ -659,9 +657,11 @@ void InputHandler::requestCheckingOfString(PassRefPtr<WebCore::TextCheckingReque
     // This should still take transactionId as a parameter to maintain the same behavior as if InputMethodSupport
     // were to cancel a request during processing.
     if (m_processingTransactionId == -1) { // Error before sending request to input service.
-        m_request->didCancel();
+        spellCheckRequest->didCancel();
         return;
     }
+
+    m_request = spellCheckRequest;
 }
 
 void InputHandler::spellCheckingRequestCancelled(int32_t transactionId)
@@ -672,7 +672,10 @@ void InputHandler::spellCheckingRequestCancelled(int32_t transactionId)
         m_processingTransactionId,
         transactionId == m_processingTransactionId ? "" : "We are out of sync with input service.");
 
-    m_request->didCancel();
+    if (m_request) {
+        m_request->didCancel();
+        m_request = 0;
+    }
     m_processingTransactionId = -1;
 }
 
@@ -680,13 +683,20 @@ void InputHandler::spellCheckingRequestProcessed(int32_t transactionId, spannabl
 {
     SpellingLog(Platform::LogLevelWarn,
         "InputHandler::spellCheckingRequestProcessed Expected transaction id %d, received %d. %s",
-        transactionId,
         m_processingTransactionId,
+        transactionId,
         transactionId == m_processingTransactionId ? "" : "We are out of sync with input service.");
 
-    if (!spannableString || !isActiveTextEdit() || !DOMSupport::elementHasContinuousSpellCheckingEnabled(m_currentFocusElement) || !m_processingTransactionId) {
+    if (!spannableString
+        || !isActiveTextEdit()
+        || !DOMSupport::elementHasContinuousSpellCheckingEnabled(m_currentFocusElement)
+        || !m_processingTransactionId
+        || !m_request) {
         SpellingLog(Platform::LogLevelWarn, "InputHandler::spellCheckingRequestProcessed Cancelling request with transactionId %d.", transactionId);
-        m_request->didCancel();
+        if (m_request) {
+            m_request->didCancel();
+            m_request = 0;
+        }
         m_processingTransactionId = -1;
         return;
     }
@@ -707,6 +717,7 @@ void InputHandler::spellCheckingRequestProcessed(int32_t transactionId, spannabl
             break;
         if (span->end < span->start) {
             m_request->didCancel();
+            m_request = 0;
             return;
         }
         if (span->attributes_mask & MISSPELLED_WORD_ATTRIB) {
@@ -725,6 +736,7 @@ void InputHandler::spellCheckingRequestProcessed(int32_t transactionId, spannabl
     free(spannableString);
 
     m_request->didSucceed(results);
+    m_request = 0;
 }
 
 SpellChecker* InputHandler::getSpellChecker()
@@ -1114,14 +1126,26 @@ void InputHandler::setElementFocused(Element* element)
         return;
 
     // Spellcheck the field in its entirety.
-    const VisibleSelection visibleSelection = DOMSupport::visibleSelectionForFocusedBlock(element);
-    m_spellingHandler->spellCheckTextBlock(visibleSelection, TextCheckingProcessBatch);
+    spellCheckTextBlock(element);
 
 #ifdef ENABLE_SPELLING_LOG
     SpellingLog(Platform::LogLevelInfo, "InputHandler::setElementFocused Spellchecking the field increased the total time to focus to %f seconds.", timer.elapsed());
 #endif
 }
 
+void InputHandler::spellCheckTextBlock(Element* element)
+{
+    if (!element) {
+        // Fall back to a valid focused element.
+        if (!m_currentFocusElement)
+            return;
+
+        element = m_currentFocusElement.get();
+    }
+    const VisibleSelection visibleSelection = DOMSupport::visibleSelectionForFocusedBlock(element);
+    m_spellingHandler->spellCheckTextBlock(visibleSelection, TextCheckingProcessBatch);
+}
+
 bool InputHandler::shouldSpellCheckElement(const Element* element) const
 {
     DOMSupport::AttributeState spellCheckAttr = DOMSupport::elementSupportsSpellCheck(element);
@@ -1141,6 +1165,11 @@ bool InputHandler::shouldSpellCheckElement(const Element* element) const
 void InputHandler::stopPendingSpellCheckRequests()
 {
     m_spellingHandler->setSpellCheckActive(false);
+    // Prevent response from propagating through
+    m_processingTransactionId = 0;
+    // Clear the pending queue as well
+    if (m_request)
+        m_request->setCheckerAndSequence(getSpellChecker(), -1);
 }
 
 void InputHandler::redrawSpellCheckDialogIfRequired(const bool shouldMoveDialog)
index d029a6a..1892ba9 100644 (file)
@@ -146,10 +146,11 @@ public:
     int32_t setComposingText(spannable_string_t*, int32_t relativeCursorPosition);
     int32_t commitText(spannable_string_t*, int32_t relativeCursorPosition);
 
-    void requestCheckingOfString(PassRefPtr<WebCore::TextCheckingRequest>);
+    void requestCheckingOfString(PassRefPtr<WebCore::SpellCheckRequest>);
     void spellCheckingRequestProcessed(int32_t transactionId, spannable_string_t*);
     void spellCheckingRequestCancelled(int32_t transactionId);
     void stopPendingSpellCheckRequests();
+    void spellCheckTextBlock(WebCore::Element* = 0);
 
     bool shouldRequestSpellCheckingOptionsForPoint(const Platform::IntPoint& documentContentPosition, const WebCore::Element*, imf_sp_text_t&);
     void requestSpellingCheckingOptions(imf_sp_text_t&, WebCore::IntSize& screenOffset, const bool shouldMoveDialog = false);
@@ -251,7 +252,7 @@ private:
     PendingKeyboardStateChange m_pendingKeyboardVisibilityChange;
     bool m_delayKeyboardVisibilityChange;
 
-    RefPtr<WebCore::TextCheckingRequest> m_request;
+    RefPtr<WebCore::SpellCheckRequest> m_request;
     int32_t m_processingTransactionId;
 
     bool m_shouldNotifyWebView;