[GTK] REGRESSION(r116135): Keys that confirm composition trigger a default action
authormrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 May 2012 18:37:22 +0000 (18:37 +0000)
committermrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 May 2012 18:37:22 +0000 (18:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=86925

Reviewed by Gustavo Noronha Silva.

Source/WebCore:

No new tests. Creating tests for GTK+ input methods is quite difficult, as they
can differ from machine to machine. It's also hard to simulate composition
sequences.

* platform/gtk/CompositionResults.h: Instead of holding composition results, just
keep a flag describing whether or not this event had results.
(CompositionResults):

Source/WebKit/gtk:

Instead of sending composition results with a keydown event, simply send whether
or not the event had results. Activate the results after processing the event. This
allows for input methods to modify the DOM after the keydown event and to still
prevent the default action during handleInputMethodKeydown.

* WebCoreSupport/EditorClientGtk.cpp:
(WebKit::EditorClient::respondToChangedSelection): This patch removes the m_updatingComposition
so we no longer have to check it here.
(WebKit::EditorClient::keyboardEventHadCompositionResults): Added.
(WebKit::EditorClient::handleInputMethodKeydown): Now prevent the default action when
the keyboard event had composition results.
(WebKit::EditorClient::EditorClient):
* WebCoreSupport/EditorClientGtk.h: Remove the m_updatingComposition member, as it's no
longer used.
* WebCoreSupport/WebViewInputMethodFilter.cpp:
(WebKit::WebViewInputMethodFilter::sendKeyEventWithCompositionResults): Now send the
composition results after the key event.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/gtk/CompositionResults.h
Source/WebKit/gtk/ChangeLog
Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp
Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.h
Source/WebKit/gtk/WebCoreSupport/WebViewInputMethodFilter.cpp

index 2882196..0a21e2e 100644 (file)
@@ -1,3 +1,18 @@
+2012-05-22  Martin Robinson  <mrobinson@igalia.com>
+
+        [GTK] REGRESSION(r116135): Keys that confirm composition trigger a default action
+        https://bugs.webkit.org/show_bug.cgi?id=86925
+
+        Reviewed by Gustavo Noronha Silva.
+
+        No new tests. Creating tests for GTK+ input methods is quite difficult, as they
+        can differ from machine to machine. It's also hard to simulate composition
+        sequences.
+
+        * platform/gtk/CompositionResults.h: Instead of holding composition results, just
+        keep a flag describing whether or not this event had results.
+        (CompositionResults):
+
 2012-05-22  Abhishek Arya  <inferno@chromium.org>
 
         Assertion failure (toRenderBox() called on a RenderInline) beneath RenderBlock::blockBeforeWithinSelectionRoot()
index 87f5bf7..c45f23b 100644 (file)
@@ -32,38 +32,32 @@ namespace WebCore {
 
 struct CompositionResults {
     CompositionResults()
-        : preeditCursorOffset(0)
+        : associatedWithPendingCompositionUpdate(false)
     {
     }
 
     CompositionResults(String simpleString)
         : simpleString(simpleString)
-        , preeditCursorOffset(0)
+        , associatedWithPendingCompositionUpdate(false)
     {
     }
 
-    CompositionResults(String confirmedComposition, String preedit, int preeditCursorOffset)
-        : confirmedComposition(confirmedComposition)
-        , preedit(preedit)
-        , preeditCursorOffset(preeditCursorOffset)
+    enum ResultsIndicator { WillSendCompositionResultsSoon };
+    CompositionResults(ResultsIndicator)
+        : associatedWithPendingCompositionUpdate(true)
     {
     }
 
     bool compositionUpdated() const
     {
-        return !confirmedComposition.isNull() || !preedit.isNull();
+        return associatedWithPendingCompositionUpdate;
     }
 
     // Some simple input methods return a string for all keyboard events. This
     // value should be treated as the string representation of the keycode.
     String simpleString;
 
-    // If the input method had a "real" composition it will be stored here.
-    String confirmedComposition;
-
-    // The event may have caused the preedit to update.
-    String preedit;
-    int preeditCursorOffset;
+    bool associatedWithPendingCompositionUpdate;
 };
 
 }
index 2741c51..1bde281 100644 (file)
@@ -1,3 +1,28 @@
+2012-05-22  Martin Robinson  <mrobinson@igalia.com>
+
+        [GTK] REGRESSION(r116135): Keys that confirm composition trigger a default action
+        https://bugs.webkit.org/show_bug.cgi?id=86925
+
+        Reviewed by Gustavo Noronha Silva.
+
+        Instead of sending composition results with a keydown event, simply send whether
+        or not the event had results. Activate the results after processing the event. This
+        allows for input methods to modify the DOM after the keydown event and to still
+        prevent the default action during handleInputMethodKeydown.
+
+        * WebCoreSupport/EditorClientGtk.cpp:
+        (WebKit::EditorClient::respondToChangedSelection): This patch removes the m_updatingComposition
+        so we no longer have to check it here.
+        (WebKit::EditorClient::keyboardEventHadCompositionResults): Added.
+        (WebKit::EditorClient::handleInputMethodKeydown): Now prevent the default action when
+        the keyboard event had composition results.
+        (WebKit::EditorClient::EditorClient):
+        * WebCoreSupport/EditorClientGtk.h: Remove the m_updatingComposition member, as it's no
+        longer used.
+        * WebCoreSupport/WebViewInputMethodFilter.cpp:
+        (WebKit::WebViewInputMethodFilter::sendKeyEventWithCompositionResults): Now send the
+        composition results after the key event.
+
 2012-05-22  Zan Dobersek  <zandobersek@gmail.com>
 
         [Gtk][LayoutTests] Repaint the complete WebKitWebView before dumping pixel results
index 0e52564..ed980d2 100644 (file)
@@ -262,7 +262,7 @@ void EditorClient::respondToChangedSelection(Frame* frame)
     setSelectionPrimaryClipboardIfNeeded(m_webView);
 #endif
 
-    if (m_updatingComposition || !frame->editor()->hasComposition() || frame->editor()->ignoreCompositionSelectionChange())
+    if (!frame->editor()->hasComposition() || frame->editor()->ignoreCompositionSelectionChange())
         return;
 
     unsigned start;
@@ -418,7 +418,7 @@ bool EditorClient::executePendingEditorCommands(Frame* frame, bool allowTextInse
     return success;
 }
 
-bool EditorClient::handleInputMethodKeyboardEvent(KeyboardEvent* event)
+static bool keyboardEventHadCompositionResults(KeyboardEvent* event)
 {
     if (event->type() != eventNames().keydownEvent)
         return false;
@@ -427,33 +427,7 @@ bool EditorClient::handleInputMethodKeyboardEvent(KeyboardEvent* event)
     if (!platformEvent)
         return false;
 
-    Frame* frame = core(m_webView)->focusController()->focusedOrMainFrame();
-    if (!frame || !frame->editor()->canEdit())
-        return false;
-
-    const CompositionResults& compositionResults = platformEvent->compositionResults();
-    if (!compositionResults.compositionUpdated())
-        return false;
-
-    m_updatingComposition = true;
-
-    // PlatformKeyboardEvent returns an empty string when there are composition results.
-    // That prevents the delivery of keypress, which is the behavior we want for composition
-    // events. See EventHandler::keyEvent. 
-    ASSERT(platformEvent->text().isNull());
-
-    if (!compositionResults.confirmedComposition.isNull())
-        frame->editor()->confirmComposition(compositionResults.confirmedComposition);
-
-    String preedit = compositionResults.preedit;
-    if (!preedit.isNull()) {
-        Vector<CompositionUnderline> underlines;
-        underlines.append(CompositionUnderline(0, preedit.length(), Color(1, 1, 1), false));
-        frame->editor()->setComposition(preedit, underlines, compositionResults.preeditCursorOffset, compositionResults.preeditCursorOffset);
-    }
-
-    m_updatingComposition = false;
-    return true;
+    return platformEvent->compositionResults().compositionUpdated();
 }
 
 void EditorClient::handleKeyboardEvent(KeyboardEvent* event)
@@ -467,7 +441,7 @@ void EditorClient::handleKeyboardEvent(KeyboardEvent* event)
     if (!platformEvent)
         return;
 
-    if (handleInputMethodKeyboardEvent(event))
+    if (keyboardEventHadCompositionResults(event))
         return;
 
     KeyBindingTranslator::EventType type = event->type() == eventNames().keydownEvent ?
@@ -519,6 +493,9 @@ void EditorClient::handleInputMethodKeydown(KeyboardEvent* event)
     // Input method results are handled in handleKeyboardEvent, so that we can wait
     // to trigger composition updates until after the keydown event handler. This better
     // matches other browsers.
+    const PlatformKeyboardEvent* platformEvent = event->keyEvent();
+    if (platformEvent && platformEvent->compositionResults().compositionUpdated())
+        event->preventDefault();
 }
 
 EditorClient::EditorClient(WebKitWebView* webView)
@@ -528,7 +505,6 @@ EditorClient::EditorClient(WebKitWebView* webView)
 #endif
     , m_webView(webView)
     , m_smartInsertDeleteEnabled(false)
-    , m_updatingComposition(false)
 {
 }
 
index 7f6173e..a1acaa8 100644 (file)
@@ -135,8 +135,6 @@ class EditorClient : public WebCore::EditorClient {
         virtual bool shouldShowUnicodeMenu();
 
     private:
-        bool handleInputMethodKeyboardEvent(WebCore::KeyboardEvent*);
-
 #if ENABLE(SPELLCHECK)
         TextCheckerClientGtk m_textCheckerClient;
 #else
@@ -146,7 +144,6 @@ class EditorClient : public WebCore::EditorClient {
         WebCore::KeyBindingTranslator m_keyBindingTranslator;
         Vector<WTF::String> m_pendingEditorCommands;
         bool m_smartInsertDeleteEnabled;
-        bool m_updatingComposition;
     };
 }
 
index 35ca23b..2700d4a 100644 (file)
@@ -60,10 +60,15 @@ bool WebViewInputMethodFilter::sendSimpleKeyEvent(GdkEventKey* event, WTF::Strin
 
 bool WebViewInputMethodFilter::sendKeyEventWithCompositionResults(GdkEventKey* event, ResultsToSend resultsToSend)
 {
-    PlatformKeyboardEvent platformEvent(event, CompositionResults(resultsToSend & Composition ? m_confirmedComposition : String(),
-                                                                  resultsToSend & Preedit ? m_preedit : String(),
-                                                                  m_cursorOffset));
-    return focusedOrMainFrame()->eventHandler()->keyEvent(platformEvent);
+    PlatformKeyboardEvent platformEvent(event, CompositionResults(CompositionResults::WillSendCompositionResultsSoon));
+    if (!focusedOrMainFrame()->eventHandler()->keyEvent(platformEvent))
+        return false;
+
+    if (resultsToSend & Composition && !m_confirmedComposition.isNull())
+        confirmCompositionText(m_confirmedComposition);
+    if (resultsToSend & Preedit && !m_preedit.isNull())
+        setPreedit(m_preedit, m_cursorOffset);
+    return true;
 }
 
 void WebViewInputMethodFilter::confirmCompositionText(String text)