Editor::m_compositionNode not updated on HTMLInputElement::setValue()
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Jan 2013 21:50:17 +0000 (21:50 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Jan 2013 21:50:17 +0000 (21:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=107737

Patch by Aurimas Liutikas <aurimas@chromium.org> on 2013-01-31
Reviewed by Ryosuke Niwa.

Source/WebCore:

Chromium has a bug where the IME composition did not get cancelled on JavaScript changes
to the focused editing field. Most of other WebKit ports were already doing this check
in their EditorClient::respondToChangedSelection. I took that logic and moved it to the
Editor so every port and use the same code.

An existing test editing/input/setting-input-value-cancel-ime-composition.html covers this change.
This test used to have an expectation to fail on Chromium and after this patch it will start passing.

* editing/Editor.cpp:
(WebCore::Editor::cancelCompositionIfSelectionIsInvalid):
    Adding a call that can be used by any the port to cancel the composition if it's no longer valid.
(WebCore):
* editing/Editor.h:
(Editor):

Source/WebKit/chromium:

* public/WebViewClient.h:
(WebKit::WebViewClient::didCancelCompositionOnSelectionChange):
    Adding a callback to let the WebViewClient know that the composition has been cancelled.
* src/EditorClientImpl.cpp:
(WebKit::EditorClientImpl::respondToChangedSelection):
    Adding a call composition if it is no longer valid.

Source/WebKit/efl:

* WebCoreSupport/EditorClientEfl.cpp:
(WebCore::EditorClientEfl::respondToChangedSelection):
    Adding a call to the newly refactored method.

Source/WebKit/gtk:

* WebCoreSupport/EditorClientGtk.cpp:
(WebKit::EditorClient::respondToChangedSelection):
    Adding a call to the newly refactored Editor method.

Source/WebKit/mac:

* WebView/WebHTMLView.mm:
(-[WebHTMLView _updateSelectionForInputManager]):

Source/WebKit/win:

* WebView.cpp:
(WebView::updateSelectionForIME):
    Adding a call to the newly refactored method.

LayoutTests:

* platform/chromium/TestExpectations: Removed fail expectation for the editing/input/setting-input-value-cancel-ime-composition.html since this patch fixes the bug https://bugs.webkit.org/show_bug.cgi?id=55560

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/Editor.h
Source/WebKit/chromium/ChangeLog
Source/WebKit/chromium/public/WebViewClient.h
Source/WebKit/chromium/src/EditorClientImpl.cpp
Source/WebKit/efl/ChangeLog
Source/WebKit/efl/WebCoreSupport/EditorClientEfl.cpp
Source/WebKit/gtk/ChangeLog
Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebHTMLView.mm
Source/WebKit/win/ChangeLog
Source/WebKit/win/WebView.cpp

index e4d5620..2a57a97 100644 (file)
@@ -1,3 +1,12 @@
+2013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
+
+        Editor::m_compositionNode not updated on HTMLInputElement::setValue()
+        https://bugs.webkit.org/show_bug.cgi?id=107737
+
+        Reviewed by Ryosuke Niwa.
+
+        * platform/chromium/TestExpectations: Removed fail expectation for the editing/input/setting-input-value-cancel-ime-composition.html since this patch fixes the bug https://bugs.webkit.org/show_bug.cgi?id=55560
+
 2013-01-31  Rouslan Solomakhin  <rouslan@chromium.org>
 
         [Chromium] Expect spellcheck to work for exactly-selected multi-word misspellings
index a9be372..8b0ff68 100644 (file)
@@ -1105,8 +1105,6 @@ crbug.com/64733 editing/text-iterator/findString.html [ Failure ]
 # Selection is wrong.
 crbug.com/64938 editing/selection/5354455-1.html [ Failure ]
 
-webkit.org/b/55560 editing/input/setting-input-value-cancel-ime-composition.html [ Failure ]
-
 # New test added in r82159
 crbug.com/77706 editing/spelling/grammar.html [ Failure ]
 
index d435f8f..7484f3d 100644 (file)
@@ -1,3 +1,25 @@
+2013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
+
+        Editor::m_compositionNode not updated on HTMLInputElement::setValue()
+        https://bugs.webkit.org/show_bug.cgi?id=107737
+
+        Reviewed by Ryosuke Niwa.
+
+        Chromium has a bug where the IME composition did not get cancelled on JavaScript changes
+        to the focused editing field. Most of other WebKit ports were already doing this check
+        in their EditorClient::respondToChangedSelection. I took that logic and moved it to the
+        Editor so every port and use the same code.
+
+        An existing test editing/input/setting-input-value-cancel-ime-composition.html covers this change.
+        This test used to have an expectation to fail on Chromium and after this patch it will start passing.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::cancelCompositionIfSelectionIsInvalid):
+            Adding a call that can be used by any the port to cancel the composition if it's no longer valid.
+        (WebCore):
+        * editing/Editor.h:
+        (Editor):
+
 2013-01-31  Joseph Pecoraro  <pecoraro@apple.com>
 
         Disable ENABLE_FULLSCREEN_API on iOS
index 12b5115..fb82f0f 100644 (file)
@@ -1343,6 +1343,17 @@ void Editor::cancelComposition()
     setComposition(emptyString(), CancelComposition);
 }
 
+bool Editor::cancelCompositionIfSelectionIsInvalid()
+{
+    unsigned start;
+    unsigned end;
+    if (!hasComposition() || ignoreCompositionSelectionChange() || getCompositionSelection(start, end))
+        return false;
+
+    cancelComposition();
+    return true;
+}
+
 void Editor::confirmComposition(const String& text)
 {
     setComposition(text, ConfirmComposition);
index 6d4603b..25d176a 100644 (file)
@@ -296,6 +296,7 @@ public:
     void confirmComposition();
     void confirmComposition(const String&); // if no existing composition, replaces selection
     void cancelComposition();
+    bool cancelCompositionIfSelectionIsInvalid();
     PassRefPtr<Range> compositionRange() const;
     bool getCompositionSelection(unsigned& selectionStart, unsigned& selectionEnd) const;
     bool setSelectionOffsets(int selectionStart, int selectionEnd);
index e35bd61..62661a3 100644 (file)
@@ -1,3 +1,17 @@
+2013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
+
+        Editor::m_compositionNode not updated on HTMLInputElement::setValue()
+        https://bugs.webkit.org/show_bug.cgi?id=107737
+
+        Reviewed by Ryosuke Niwa.
+
+        * public/WebViewClient.h:
+        (WebKit::WebViewClient::didCancelCompositionOnSelectionChange):
+            Adding a callback to let the WebViewClient know that the composition has been cancelled.
+        * src/EditorClientImpl.cpp:
+        (WebKit::EditorClientImpl::respondToChangedSelection):
+            Adding a call composition if it is no longer valid.
+
 2013-01-31  Mark Pilgrim  <pilgrim@chromium.org>
 
         [Chromium] Move LocalizedStrings to WebCore
index a18179c..38c66df 100644 (file)
@@ -190,6 +190,7 @@ public:
     virtual bool isSelectTrailingWhitespaceEnabled() { return true; }
 
     virtual void didBeginEditing() { }
+    virtual void didCancelCompositionOnSelectionChange() { }
     virtual void didChangeSelection(bool isSelectionEmpty) { }
     virtual void didChangeContents() { }
     virtual void didExecuteCommand(const WebString& commandName) { }
index 8d0e97a..cdcaa12 100644 (file)
@@ -268,8 +268,11 @@ void EditorClientImpl::didBeginEditing()
 void EditorClientImpl::respondToChangedSelection(Frame* frame)
 {
     if (m_webView->client()) {
-        if (frame)
+        if (frame) {
             m_webView->client()->didChangeSelection(!frame->selection()->isRange());
+            if (frame->editor()->cancelCompositionIfSelectionIsInvalid())
+                m_webView->client()->didCancelCompositionOnSelectionChange();
+        }
     }
 }
 
index c5084ba..028aea5 100644 (file)
@@ -1,3 +1,14 @@
+2013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
+
+        Editor::m_compositionNode not updated on HTMLInputElement::setValue()
+        https://bugs.webkit.org/show_bug.cgi?id=107737
+
+        Reviewed by Ryosuke Niwa.
+
+        * WebCoreSupport/EditorClientEfl.cpp:
+        (WebCore::EditorClientEfl::respondToChangedSelection):
+            Adding a call to the newly refactored method.
+
 2013-01-31  Enrica Casucci  <enrica@apple.com>
 
         WebKit2: provide new bundle APIs to allow bundle clients to be notified of pasteboard access.
index 5ab7501..9f41c83 100644 (file)
@@ -152,14 +152,7 @@ void EditorClientEfl::respondToChangedSelection(Frame* coreFrame)
     Evas_Object* webFrame = EWKPrivate::kitFrame(coreFrame);
     ewk_frame_editor_client_selection_changed(webFrame);
 
-    if (!coreFrame->editor()->hasComposition() || coreFrame->editor()->ignoreCompositionSelectionChange())
-        return;
-
-    unsigned start;
-    unsigned end;
-
-    if (!coreFrame->editor()->getCompositionSelection(start, end))
-        coreFrame->editor()->cancelComposition();
+    coreFrame->editor()->cancelCompositionIfSelectionIsInvalid();
 }
 
 void EditorClientEfl::didEndEditing()
index a2b5b9b..2630159 100644 (file)
@@ -1,3 +1,14 @@
+2013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
+
+        Editor::m_compositionNode not updated on HTMLInputElement::setValue()
+        https://bugs.webkit.org/show_bug.cgi?id=107737
+
+        Reviewed by Ryosuke Niwa.
+
+        * WebCoreSupport/EditorClientGtk.cpp:
+        (WebKit::EditorClient::respondToChangedSelection):
+            Adding a call to the newly refactored Editor method.
+
 2013-01-31  Enrica Casucci  <enrica@apple.com>
 
         WebKit2: provide new bundle APIs to allow bundle clients to be notified of pasteboard access.
index 89e883e..f24d7be 100644 (file)
@@ -262,12 +262,7 @@ void EditorClient::respondToChangedSelection(Frame* frame)
     setSelectionPrimaryClipboardIfNeeded(m_webView);
 #endif
 
-    if (!frame->editor()->hasComposition() || frame->editor()->ignoreCompositionSelectionChange())
-        return;
-
-    unsigned start;
-    unsigned end;
-    if (!frame->editor()->getCompositionSelection(start, end))
+    if (frame->editor()->cancelCompositionIfSelectionIsInvalid())
         m_webView->priv->imFilter.resetContext();
 }
 
index c9a4253..67db82a 100644 (file)
@@ -1,3 +1,13 @@
+2013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
+
+        Editor::m_compositionNode not updated on HTMLInputElement::setValue()
+        https://bugs.webkit.org/show_bug.cgi?id=107737
+
+        Reviewed by Ryosuke Niwa.
+
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView _updateSelectionForInputManager]):
+
 2013-01-31  Joseph Pecoraro  <pecoraro@apple.com>
 
         Disable ENABLE_FULLSCREEN_API on iOS
index 074b608..1345cbd 100644 (file)
@@ -6037,10 +6037,7 @@ static void extractUnderlines(NSAttributedString *string, Vector<CompositionUnde
 
     [self _updateSecureInputState];
 
-    if (!coreFrame->editor()->hasComposition())
-        return;
-
-    if (coreFrame->editor()->ignoreCompositionSelectionChange())
+    if (!coreFrame->editor()->hasComposition() || coreFrame->editor()->ignoreCompositionSelectionChange())
         return;
 
     unsigned start;
index 9edb32a..3175a2d 100644 (file)
@@ -1,3 +1,14 @@
+2013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
+
+        Editor::m_compositionNode not updated on HTMLInputElement::setValue()
+        https://bugs.webkit.org/show_bug.cgi?id=107737
+
+        Reviewed by Ryosuke Niwa.
+
+        * WebView.cpp:
+        (WebView::updateSelectionForIME):
+            Adding a call to the newly refactored method.
+
 2013-01-31  Enrica Casucci  <enrica@apple.com>
 
         WebKit2: provide new bundle APIs to allow bundle clients to be notified of pasteboard access.
index c7e76d3..880e412 100644 (file)
@@ -5481,15 +5481,11 @@ void WebView::resetIME(Frame* targetFrame)
 void WebView::updateSelectionForIME()
 {
     Frame* targetFrame = m_page->focusController()->focusedOrMainFrame();
-    if (!targetFrame || !targetFrame->editor()->hasComposition())
-        return;
     
-    if (targetFrame->editor()->ignoreCompositionSelectionChange())
+    if (!targetFrame)
         return;
 
-    unsigned start;
-    unsigned end;
-    if (!targetFrame->editor()->getCompositionSelection(start, end))
+    if (!targetFrame->editor()->cancelCompositionIfSelectionIsInvalid())
         resetIME(targetFrame);
 }