Changeset 252976 in webkit

Timestamp:

Dec 1, 2019 7:19:14 PM (4 years ago)

Author:

Fujii Hironori

Message:

[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.

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.

Location:

trunk/Source

Files:

• WebKit/ChangeLog (modified) (1 diff)
• WebKit/Shared/NativeWebKeyboardEvent.h (modified) (3 diffs)
• WebKit/Shared/win/NativeWebKeyboardEventWin.cpp (modified) (1 diff)
• WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• WebKit/UIProcess/WebPageProxy.h (modified) (1 diff)
• WebKit/UIProcess/win/WebPageProxyWin.cpp (modified) (2 diffs)
• WebKit/UIProcess/win/WebView.cpp (modified) (1 diff)
• WebKitLegacy/win/ChangeLog (modified) (1 diff)
• WebKitLegacy/win/WebView.cpp (modified) (4 diffs)

trunk/Source/WebKit/ChangeLog

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

trunk/Source/WebKit/Shared/NativeWebKeyboardEvent.h

@@ -77 +77 @@
     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
 
@@ -91 +91 @@
 #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; }
@@ -107 +108 @@
 #elif PLATFORM(WIN)
     MSG m_nativeEvent;
+    Vector<MSG> m_pendingCharEvents;
 #endif
 };

trunk/Source/WebKit/Shared/win/NativeWebKeyboardEventWin.cpp

@@ -34 +34 @@
 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))
 {
 }

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -6576 +6576 @@
         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) {

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -866 +866 @@
     bool isProcessingKeyboardEvents() const;
     void handleKeyboardEvent(const NativeWebKeyboardEvent&);
+#if PLATFORM(WIN)
+    void dispatchPendingCharEvents(const NativeWebKeyboardEvent&);
+#endif
 
 #if ENABLE(MAC_GESTURE_EVENTS)

trunk/Source/WebKit/UIProcess/win/WebPageProxyWin.cpp

@@ -28 +28 @@
 #include "WebPageProxy.h"
 
+#include "NativeWebKeyboardEvent.h"
 #include "PageClientImpl.h"
 #include <WebCore/SearchPopupMenuDB.h>
@@ -87 +88 @@
 #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

trunk/Source/WebKit/UIProcess/win/WebView.cpp

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

trunk/Source/WebKitLegacy/win/ChangeLog

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

trunk/Source/WebKitLegacy/win/WebView.cpp

@@ -2377 +2377 @@
     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);
@@ -2385 +2394 @@
         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.
@@ -2403 +2406 @@
 
     // 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:
@@ -2439 +2443 @@
             break;
         default:
-            return false;
-    }
-
-    return frame.eventHandler().scrollRecursively(direction, granularity);
+            willScroll = false;
+            break;
+    }
+
+    if (willScroll) {
+        handled = frame.eventHandler().scrollRecursively(direction, granularity);
+        if (handled)
+            return true;
+    }
+
+    for (auto& msg : pendingCharEvents)
+        DispatchMessage(&msg);
+    return false;
 }