[Win] Retrieve all following WM_CHAR events at the beginning of processing WM_KEYDOWN...
authorHironori.Fujii@sony.com <Hironori.Fujii@sony.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Dec 2019 03:19:14 +0000 (03:19 +0000)
committerHironori.Fujii@sony.com <Hironori.Fujii@sony.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 2 Dec 2019 03:19:14 +0000 (03:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=204694

Reviewed by Ross Kirsling.

Source/WebKit:

In Windows ports, WM_KEYDOWN dispatches keydown event, and
WM_CHAR dispatches keypress event. If a keydown event is canceled
by calling preventDefault, the following corresponding keypress
events shouldn't be dispatched.

WebKit1 implemented it by removing WM_CHAR events if the keydown
event is consumed. However, WebKit2 can't do so because WebKit2
processes key events asynchronously. Thus, retrieve all following
WM_CHAR events, and dispatch them after processing the keydown
and if it is not consumed.

In addition to that, retrieving following WM_CHAR events is needed
to fix Bug 204672 because the events are needed for 'key' property
of keydown KeyboardEvent for dead key combination.

Gecko and Chromium also implements 'key' property in the same approach.

Test: Covered by existing fast/events/inputText-never-fired-on-keydown-cancel.html and fast/events/keydown-keypress-preventDefault.html

* Shared/NativeWebKeyboardEvent.h: Added m_pendingCharEvents as Vector<MSG>.
(WebKit::NativeWebKeyboardEvent::pendingCharEvents const):
* Shared/win/NativeWebKeyboardEventWin.cpp:
(WebKit::NativeWebKeyboardEvent::NativeWebKeyboardEvent):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didReceiveEvent):
* UIProcess/WebPageProxy.h:
* UIProcess/win/WebPageProxyWin.cpp:
(WebKit::WebPageProxy::dispatchPendingCharEvents):
* UIProcess/win/WebView.cpp:
(WebKit::WebView::onKeyEvent):

Source/WebKitLegacy/win:

* WebView.cpp:
(WebView::keyDown): Added a variable pendingCharEvents of
Vector<MSG> to preserve following WM_CHAR events. Dispatch them if
the WM_KEYDOWN isn't consumed.

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

Source/WebKit/ChangeLog
Source/WebKit/Shared/NativeWebKeyboardEvent.h
Source/WebKit/Shared/win/NativeWebKeyboardEventWin.cpp
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/win/WebPageProxyWin.cpp
Source/WebKit/UIProcess/win/WebView.cpp
Source/WebKitLegacy/win/ChangeLog
Source/WebKitLegacy/win/WebView.cpp

index 93e2e39..e4f5511 100644 (file)
@@ -1,3 +1,41 @@
+2019-12-01  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        [Win] Retrieve all following WM_CHAR events at the beginning of processing WM_KEYDOWN event
+        https://bugs.webkit.org/show_bug.cgi?id=204694
+
+        Reviewed by Ross Kirsling.
+
+        In Windows ports, WM_KEYDOWN dispatches keydown event, and
+        WM_CHAR dispatches keypress event. If a keydown event is canceled
+        by calling preventDefault, the following corresponding keypress
+        events shouldn't be dispatched.
+
+        WebKit1 implemented it by removing WM_CHAR events if the keydown
+        event is consumed. However, WebKit2 can't do so because WebKit2
+        processes key events asynchronously. Thus, retrieve all following
+        WM_CHAR events, and dispatch them after processing the keydown
+        and if it is not consumed.
+
+        In addition to that, retrieving following WM_CHAR events is needed
+        to fix Bug 204672 because the events are needed for 'key' property
+        of keydown KeyboardEvent for dead key combination.
+
+        Gecko and Chromium also implements 'key' property in the same approach.
+
+        Test: Covered by existing fast/events/inputText-never-fired-on-keydown-cancel.html and fast/events/keydown-keypress-preventDefault.html
+
+        * Shared/NativeWebKeyboardEvent.h: Added m_pendingCharEvents as Vector<MSG>.
+        (WebKit::NativeWebKeyboardEvent::pendingCharEvents const):
+        * Shared/win/NativeWebKeyboardEventWin.cpp:
+        (WebKit::NativeWebKeyboardEvent::NativeWebKeyboardEvent):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didReceiveEvent):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/win/WebPageProxyWin.cpp:
+        (WebKit::WebPageProxy::dispatchPendingCharEvents):
+        * UIProcess/win/WebView.cpp:
+        (WebKit::WebView::onKeyEvent):
+
 2019-11-30  Tim Horton  <timothy_horton@apple.com>
 
         Make CompositeOperator and BlendMode encodable
index 6f4e29b..05938ef 100644 (file)
@@ -76,7 +76,7 @@ public:
 #elif USE(LIBWPE)
     NativeWebKeyboardEvent(struct wpe_input_keyboard_event*);
 #elif PLATFORM(WIN)
-    NativeWebKeyboardEvent(HWND, UINT message, WPARAM, LPARAM);
+    NativeWebKeyboardEvent(HWND, UINT message, WPARAM, LPARAM, Vector<MSG>&& pendingCharEvents);
 #endif
 
 #if USE(APPKIT)
@@ -90,6 +90,7 @@ public:
     ::WebEvent* nativeEvent() const { return m_nativeEvent.get(); }
 #elif PLATFORM(WIN)
     const MSG* nativeEvent() const { return &m_nativeEvent; }
+    const Vector<MSG>& pendingCharEvents() const { return m_pendingCharEvents; }
 #else
     const void* nativeEvent() const { return nullptr; }
 #endif
@@ -106,6 +107,7 @@ private:
     RetainPtr<::WebEvent> m_nativeEvent;
 #elif PLATFORM(WIN)
     MSG m_nativeEvent;
+    Vector<MSG> m_pendingCharEvents;
 #endif
 };
 
index 9be9292..4eef868 100644 (file)
@@ -33,9 +33,10 @@ namespace WebKit {
 
 using namespace WebCore;
 
-NativeWebKeyboardEvent::NativeWebKeyboardEvent(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
+NativeWebKeyboardEvent::NativeWebKeyboardEvent(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam, Vector<MSG>&& pendingCharEvents)
     : WebKeyboardEvent(WebEventFactory::createWebKeyboardEvent(hwnd, message, wParam, lParam))
     , m_nativeEvent(createNativeEvent(hwnd, message, wParam, lParam))
+    , m_pendingCharEvents(WTFMove(pendingCharEvents))
 {
 }
 
index 81b65c6..d06c337 100644 (file)
@@ -6575,6 +6575,11 @@ void WebPageProxy::didReceiveEvent(uint32_t opaqueType, bool handled)
 
         MESSAGE_CHECK(m_process, type == event.type());
 
+#if PLATFORM(WIN)
+        if (!handled && type == WebEvent::RawKeyDown)
+            dispatchPendingCharEvents(event);
+#endif
+
         bool canProcessMoreKeyEvents = !m_keyEventQueue.isEmpty();
         if (canProcessMoreKeyEvents) {
             LOG(KeyHandling, " UI process: sent keyEvent from didReceiveEvent");
index 488ac80..7356cce 100644 (file)
@@ -865,6 +865,9 @@ public:
 
     bool isProcessingKeyboardEvents() const;
     void handleKeyboardEvent(const NativeWebKeyboardEvent&);
+#if PLATFORM(WIN)
+    void dispatchPendingCharEvents(const NativeWebKeyboardEvent&);
+#endif
 
 #if ENABLE(MAC_GESTURE_EVENTS)
     void handleGestureEvent(const NativeWebGestureEvent&);
index bb047f2..8b267d7 100644 (file)
@@ -27,6 +27,7 @@
 #include "config.h"
 #include "WebPageProxy.h"
 
+#include "NativeWebKeyboardEvent.h"
 #include "PageClientImpl.h"
 #include <WebCore/SearchPopupMenuDB.h>
 #include <WebCore/UserAgent.h>
@@ -86,5 +87,11 @@ void WebPageProxy::setDevice(ID3D11Device1* device)
 }
 #endif
 
+void WebPageProxy::dispatchPendingCharEvents(const NativeWebKeyboardEvent& keydownEvent)
+{
+    auto& pendingCharEvents = keydownEvent.pendingCharEvents();
+    for (auto it = pendingCharEvents.rbegin(); it != pendingCharEvents.rend(); it++)
+        m_keyEventQueue.prepend(NativeWebKeyboardEvent(it->hwnd, it->message, it->wParam, it->lParam, { }));
+}
 
 } // namespace WebKit
index 9b92261..d19617b 100644 (file)
@@ -467,7 +467,16 @@ LRESULT WebView::onVerticalScroll(HWND hWnd, UINT message, WPARAM wParam, LPARAM
 
 LRESULT WebView::onKeyEvent(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam, bool& handled)
 {
-    m_page->handleKeyboardEvent(NativeWebKeyboardEvent(hWnd, message, wParam, lParam));
+    Vector<MSG> pendingCharEvents;
+    if (message == WM_KEYDOWN) {
+        MSG msg;
+        // WM_SYSCHAR events should not be removed, because WebKit is using WM_SYSCHAR for access keys and they can't be canceled.
+        while (PeekMessage(&msg, hWnd, WM_CHAR, WM_DEADCHAR, PM_REMOVE)) {
+            if (msg.message == WM_CHAR)
+                pendingCharEvents.append(msg);
+        }
+    }
+    m_page->handleKeyboardEvent(NativeWebKeyboardEvent(hWnd, message, wParam, lParam, WTFMove(pendingCharEvents)));
 
     // We claim here to always have handled the event. If the event is not in fact handled, we will
     // find out later in didNotHandleKeyEvent.
index a0c031b..5b1fe0d 100644 (file)
@@ -1,5 +1,17 @@
 2019-12-01  Fujii Hironori  <Hironori.Fujii@sony.com>
 
+        [Win] Retrieve all following WM_CHAR events at the beginning of processing WM_KEYDOWN event
+        https://bugs.webkit.org/show_bug.cgi?id=204694
+
+        Reviewed by Ross Kirsling.
+
+        * WebView.cpp:
+        (WebView::keyDown): Added a variable pendingCharEvents of
+        Vector<MSG> to preserve following WM_CHAR events. Dispatch them if
+        the WM_KEYDOWN isn't consumed.
+
+2019-12-01  Fujii Hironori  <Hironori.Fujii@sony.com>
+
         [CMake] Ninja can't build WebKitQuartzCoreAdditions of AppleWin port
         https://bugs.webkit.org/show_bug.cgi?id=204696
 
index 36dd589..006dfb8 100644 (file)
@@ -2376,6 +2376,15 @@ bool WebView::keyDown(WPARAM virtualKeyCode, LPARAM keyData, bool systemKeyDown)
 #endif
     Frame& frame = m_page->focusController().focusedOrMainFrame();
 
+    Vector<MSG> pendingCharEvents;
+    MSG msg;
+    while (PeekMessage(&msg, m_viewWindow, WM_CHAR, WM_DEADCHAR, PM_REMOVE)) {
+        // FIXME: remove WM_UNICHAR, too
+        // WM_SYSCHAR events should not be removed, because WebKit is using WM_SYSCHAR for access keys and they can't be canceled.
+        if (msg.message == WM_CHAR)
+            pendingCharEvents.append(msg);
+    }
+
     PlatformKeyboardEvent keyEvent(m_viewWindow, virtualKeyCode, keyData, PlatformEvent::RawKeyDown, systemKeyDown);
     bool handled = frame.eventHandler().keyEvent(keyEvent);
 
@@ -2384,14 +2393,8 @@ bool WebView::keyDown(WPARAM virtualKeyCode, LPARAM keyData, bool systemKeyDown)
     if (systemKeyDown && virtualKeyCode != VK_RETURN)
         return false;
 
-    if (handled) {
-        // FIXME: remove WM_UNICHAR, too
-        MSG msg;
-        // WM_SYSCHAR events should not be removed, because access keys are implemented in WebCore in WM_SYSCHAR handler.
-        if (!systemKeyDown)
-            ::PeekMessage(&msg, m_viewWindow, WM_CHAR, WM_CHAR, PM_REMOVE);
+    if (handled)
         return true;
-    }
 
     // We need to handle back/forward using either Ctrl+Left/Right Arrow keys.
     // FIXME: This logic should probably be in EventHandler::defaultArrowEventHandler().
@@ -2402,8 +2405,9 @@ bool WebView::keyDown(WPARAM virtualKeyCode, LPARAM keyData, bool systemKeyDown)
         return m_page->backForward().goBack();
 
     // Need to scroll the page if the arrow keys, pgup/dn, or home/end are hit.
-    ScrollDirection direction;
-    ScrollGranularity granularity;
+    bool willScroll = true;
+    ScrollDirection direction { };
+    ScrollGranularity granularity { };
     switch (virtualKeyCode) {
         case VK_LEFT:
             granularity = ScrollByLine;
@@ -2438,10 +2442,19 @@ bool WebView::keyDown(WPARAM virtualKeyCode, LPARAM keyData, bool systemKeyDown)
             direction = ScrollDown;
             break;
         default:
-            return false;
+            willScroll = false;
+            break;
     }
 
-    return frame.eventHandler().scrollRecursively(direction, granularity);
+    if (willScroll) {
+        handled = frame.eventHandler().scrollRecursively(direction, granularity);
+        if (handled)
+            return true;
+    }
+
+    for (auto& msg : pendingCharEvents)
+        DispatchMessage(&msg);
+    return false;
 }
 
 bool WebView::keyPress(WPARAM charCode, LPARAM keyData, bool systemKeyDown)