Changeset 83633 in webkit

Timestamp:

Apr 12, 2011 1:44:17 PM (13 years ago)

Author:

jeffm@apple.com

Message:

2011-04-12 Jeff Miller <​jeffm@apple.com>

Reviewed by Adam Roben.

WebKit2: Pressing Tab in Web Inspector's console does not cycle through completion options
​https://bugs.webkit.org/show_bug.cgi?id=56020

Don't call TranslateMessage() in the MiniBrowser or TestWebKitAPI for key messages destined for a WebKit2 view,
since WebKit will do this for us. If we didn't do this, TranslateMessage() would be called twice,
which would generate two characters for every keypress (for example). I didn't bother doing this for
WebKitTestRunner, since it doesn't get any WM_KEYDOWN events.

Add new WebKit2/TranslateMessageGeneratesWMChar test to test expected TranslateMessage() behavior.

• MiniBrowser/win/main.cpp: (shouldTranslateMessage): Added. (_tWinMain): Don't call TranslateMessage() unless shouldTranslateMessage() says to.
• TestWebKitAPI/PlatformUtilities.h: Added shouldTranslateMessage() on Windows.
• TestWebKitAPI/PlatformWebView.h: Added simulateAKeyDown().
• TestWebKitAPI/Tests/WebKit2/win/TranslateMessageGeneratesWMChar.cpp: Added. (TestWebKitAPI::didNotHandleKeyEventCallback): Added. (TestWebKitAPI::runAndWatchForWMChar): Added. (TestWebKitAPI::TEST): Added.
• TestWebKitAPI/win/PlatformUtilitiesWin.cpp: (TestWebKitAPI::Util::run): Don't call TranslateMessage() unless shouldTranslateMessage() says to. (TestWebKitAPI::Util::shouldTranslateMessage): Added.
• TestWebKitAPI/win/PlatformWebViewWin.cpp: (TestWebKitAPI::PlatformWebView::simulateAKeyDown): Added.
• TestWebKitAPI/win/TestWebKitAPI.vcproj: Added TranslateMessageGeneratesWMChar.cpp.

2011-04-12 Jeff Miller <​jeffm@apple.com>

Reviewed by Adam Roben.

WebKit2: Pressing Tab in Web Inspector's console does not cycle through completion options
​https://bugs.webkit.org/show_bug.cgi?id=56020

Safari was always calling TranslateMessage() on key events since it has no way to know whether
WebKit handled the event without a PageUIClient (which Safari only installs on pages inside
a Safari window), which was generating a WM_CHAR message containing the tab in this case. The fix
is for Safari to never call TranslateMessage() on key events outside of a Safari window, but this
means the WebPageProxy needs to do this for unhandled key events if there is no didNotHandleKeyEvent
callback in the PageUIClient.

• UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::didReceiveEvent): Call TranslateMessage() on Windows for unhandled key events that can't be handled by the PageUIClient.
• UIProcess/WebUIClient.cpp: (WebKit::WebUIClient::canNotHandleKeyEvent): Added.
• UIProcess/WebUIClient.h: Added canNotHandleKeyEvent().

Location:

trunk

Files:

• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Source/WebKit2/UIProcess/WebUIClient.cpp (modified) (1 diff)
• Source/WebKit2/UIProcess/WebUIClient.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/MiniBrowser/win/main.cpp (modified) (2 diffs)
• Tools/TestWebKitAPI/PlatformUtilities.h (modified) (2 diffs)
• Tools/TestWebKitAPI/PlatformWebView.h (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2/win/TranslateMessageGeneratesWMChar.cpp (added)
• Tools/TestWebKitAPI/win/PlatformUtilitiesWin.cpp (modified) (1 diff)
• Tools/TestWebKitAPI/win/PlatformWebViewWin.cpp (modified) (1 diff)
• Tools/TestWebKitAPI/win/TestWebKitAPI.vcproj (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2011-04-12  Jeff Miller  <jeffm@apple.com>
+
+        Reviewed by Adam Roben.
+
+        WebKit2: Pressing Tab in Web Inspector's console does not cycle through completion options
+        https://bugs.webkit.org/show_bug.cgi?id=56020
+
+        Safari was always calling TranslateMessage() on key events since it has no way to know whether
+        WebKit handled the event without a PageUIClient (which Safari only installs on pages inside
+        a Safari window), which was generating a WM_CHAR message containing the tab in this case.  The fix
+        is for Safari to never call TranslateMessage() on key events outside of a Safari window, but this
+        means the WebPageProxy needs to do this for unhandled key events if there is no didNotHandleKeyEvent
+        callback in the PageUIClient.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didReceiveEvent): Call TranslateMessage() on Windows for unhandled key events that can't be handled by the PageUIClient.
+        * UIProcess/WebUIClient.cpp:
+        (WebKit::WebUIClient::canNotHandleKeyEvent): Added.
+        * UIProcess/WebUIClient.h: Added canNotHandleKeyEvent().
+
 2011-04-12  Alice Liu  <alice.liu@apple.com>
 

trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

@@ -2564 +2564 @@
             break;
 
-        m_uiClient.didNotHandleKeyEvent(this, event);
+        if (m_uiClient.implementsDidNotHandleKeyEvent())
+            m_uiClient.didNotHandleKeyEvent(this, event);
+#if PLATFORM(WIN)
+        else
+            ::TranslateMessage(event.nativeEvent());
+#endif
         break;
     }

trunk/Source/WebKit2/UIProcess/WebUIClient.cpp

@@ -156 +156 @@
 }
 
+bool WebUIClient::implementsDidNotHandleKeyEvent() const
+{
+    return m_client.didNotHandleKeyEvent;
+}
+
 void WebUIClient::didNotHandleKeyEvent(WebPageProxy* page, const NativeWebKeyboardEvent& event)
 {

trunk/Source/WebKit2/UIProcess/WebUIClient.h

@@ -67 +67 @@
     void mouseDidMoveOverElement(WebPageProxy*, WebEvent::Modifiers, APIObject*);
     void missingPluginButtonClicked(WebPageProxy*, const String& mimeType, const String& url, const String& pluginsPageURL);
+    
+    bool implementsDidNotHandleKeyEvent() const;
     void didNotHandleKeyEvent(WebPageProxy*, const NativeWebKeyboardEvent&);
 

trunk/Tools/ChangeLog

@@ +1 @@
+2011-04-12  Jeff Miller  <jeffm@apple.com>
+
+        Reviewed by Adam Roben.
+
+        WebKit2: Pressing Tab in Web Inspector's console does not cycle through completion options
+        https://bugs.webkit.org/show_bug.cgi?id=56020
+
+        Don't call TranslateMessage() in the MiniBrowser or TestWebKitAPI for key messages destined for a WebKit2 view,
+        since WebKit will do this for us.  If we didn't do this, TranslateMessage() would be called twice,
+        which would generate two characters for every keypress (for example).  I didn't bother doing this for
+        WebKitTestRunner, since it doesn't get any WM_KEYDOWN events.
+        
+        Add new WebKit2/TranslateMessageGeneratesWMChar test to test expected TranslateMessage() behavior.
+
+        * MiniBrowser/win/main.cpp:
+        (shouldTranslateMessage): Added.
+        (_tWinMain): Don't call TranslateMessage() unless shouldTranslateMessage() says to.
+        * TestWebKitAPI/PlatformUtilities.h: Added shouldTranslateMessage() on Windows.
+        * TestWebKitAPI/PlatformWebView.h: Added simulateAKeyDown().
+        * TestWebKitAPI/Tests/WebKit2/win/TranslateMessageGeneratesWMChar.cpp: Added.
+        (TestWebKitAPI::didNotHandleKeyEventCallback): Added.
+        (TestWebKitAPI::runAndWatchForWMChar): Added.
+        (TestWebKitAPI::TEST): Added.
+        * TestWebKitAPI/win/PlatformUtilitiesWin.cpp:
+        (TestWebKitAPI::Util::run): Don't call TranslateMessage() unless shouldTranslateMessage() says to.
+        (TestWebKitAPI::Util::shouldTranslateMessage): Added.
+        * TestWebKitAPI/win/PlatformWebViewWin.cpp:
+        (TestWebKitAPI::PlatformWebView::simulateAKeyDown): Added.
+        * TestWebKitAPI/win/TestWebKitAPI.vcproj: Added TranslateMessageGeneratesWMChar.cpp.
+
 2011-04-12  Dirk Pranke  <dpranke@chromium.org>
 

trunk/Tools/MiniBrowser/win/main.cpp

@@ -42 +42 @@
 #pragma comment(linker, "/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='" PROCESSORARCHITECTURE "' publicKeyToken='6595b64144ccf1df' language='*'\"")
 
+static bool shouldTranslateMessage(const MSG& msg)
+{
+    // Only these four messages are actually translated by ::TranslateMessage or ::TranslateAccelerator.
+    // It's useless (though harmless) to call those functions for other messages, so we always allow other messages to be translated.
+    if (msg.message != WM_KEYDOWN && msg.message != WM_SYSKEYDOWN && msg.message != WM_KEYUP && msg.message != WM_SYSKEYUP)
+        return true;
+    
+    wchar_t className[256];
+    if (!::GetClassNameW(msg.hwnd, className, ARRAYSIZE(className)))
+        return true;
+
+    // Don't call TranslateMessage() on key events destined for a WebKit2 view, WebKit will do this if it doesn't handle the message.
+    // It would be nice to use some API here instead of hard-coding the window class name.
+    return wcscmp(className, L"WebKit2WebViewWindowClass");
+}
+
 int WINAPI _tWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPTSTR lpstrCmdLine, int nCmdShow)
 {
@@ -53 +69 @@
         if (result == -1)
             break;
-        ::TranslateMessage(&message);
+        
+        if (shouldTranslateMessage(message))
+            ::TranslateMessage(&message);
 
         if (!MiniBrowser::shared().handleMessage(&message))

trunk/Tools/TestWebKitAPI/PlatformUtilities.h

@@ -29 +29 @@
 #include <WebKit2/WebKit2.h>
 #include <WebKit2/WKRetainPtr.h>
+#include <wtf/Platform.h>
 #include <string>
 
@@ -36 +37 @@
 // Runs a platform runloop until the 'done' is true. 
 void run(bool* done);
+
+#if PLATFORM(WIN)
+bool shouldTranslateMessage(const MSG&);
+#endif
 
 void sleep(double seconds);

trunk/Tools/TestWebKitAPI/PlatformWebView.h

@@ -66 +66 @@
 
 #if PLATFORM(WIN)
+    void simulateAKeyDown();
     void setParentWindowMessageObserver(WindowMessageObserver* observer) { m_parentWindowMessageObserver = observer; }
 #endif

trunk/Tools/TestWebKitAPI/win/PlatformUtilitiesWin.cpp

@@ -50 +50 @@
         if (!result || result == -1)
             break;
-        ::TranslateMessage(&msg);
+        
+        if (shouldTranslateMessage(msg))
+            ::TranslateMessage(&msg);
         ::DispatchMessage(&msg);
     }
+}
+
+bool shouldTranslateMessage(const MSG& msg)
+{
+    // Only these four messages are actually translated by ::TranslateMessage or ::TranslateAccelerator.
+    // It's useless (though harmless) to call those functions for other messages, so we always allow other messages to be translated.
+    if (msg.message != WM_KEYDOWN && msg.message != WM_SYSKEYDOWN && msg.message != WM_KEYUP && msg.message != WM_SYSKEYUP)
+        return true;
+    
+    wchar_t className[256];
+    if (!::GetClassNameW(msg.hwnd, className, ARRAYSIZE(className)))
+        return true;
+
+    // Don't call TranslateMessage() on key events destined for a WebKit2 view, WebKit will do this if it doesn't handle the message.
+    // It would be nice to use some API here instead of hard-coding the window class name.
+    return wcscmp(className, L"WebKit2WebViewWindowClass");
 }
 

trunk/Tools/TestWebKitAPI/win/PlatformWebViewWin.cpp

@@ -93 +93 @@
 }
 
+void PlatformWebView::simulateAKeyDown()
+{
+    HWND window = WKViewGetWindow(m_view);
+    
+    // These values match what happens when you press the 'A' key in Notepad, as observed by Spy++.
+    ::SendMessageW(window, WM_KEYDOWN, 'A', (1 << repeatCountBitOffset) | (30 << scanCodeBitOffset));
+}
+
 void PlatformWebView::simulateAltKeyPress()
 {

trunk/Tools/TestWebKitAPI/win/TestWebKitAPI.vcproj

@@ -560 +560 @@
                                         </File>
                                         <File
+                                                RelativePath="..\Tests\WebKit2\win\TranslateMessageGeneratesWMChar.cpp"
+                                                >
+                                        </File>
+                                        <File
                                                 RelativePath="..\Tests\WebKit2\win\WMCloseCallsUIClientClose.cpp"
                                                 >