Summary: | Need API to tell a WebKit2 client application that a key event was not handled | ||
---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> |
Component: | WebKit2 | Assignee: | Adam Roben (:aroben) <aroben> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | andersca, ap, enrica, kenneth, sam, webkit-ews, webkit.review.bot |
Priority: | P2 | Keywords: | InRadar |
Version: | 528+ (Nightly build) | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
Description
Adam Roben (:aroben)
2010-09-27 15:04:08 PDT
Created attachment 69051 [details]
Tell the UI client when a key event is not handled
Attachment 69051 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/Shared/win/NativeWebKeyboardEventWin.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/API/C/WKPage.h:166: Extra space between WKPageDidNotHandleKeyEventCallback and didNotHandleKeyEvent [whitespace/declaration] [3]
WebKit2/Shared/qt/NativeWebKeyboardEventQt.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/win/WebView.cpp:30: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 4 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 69051 [details] did not build on qt: Build output: http://queues.webkit.org/results/4132026 Created attachment 69054 [details]
Tell the UI client when a key event is not handled
Attachment 69054 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/Shared/win/NativeWebKeyboardEventWin.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/API/C/WKPage.h:166: Extra space between WKPageDidNotHandleKeyEventCallback and didNotHandleKeyEvent [whitespace/declaration] [3]
WebKit2/Shared/qt/NativeWebKeyboardEventQt.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/win/WebView.cpp:30: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 4 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 69054 [details] did not build on qt: Build output: http://queues.webkit.org/results/4035204 A QKeyEvent is a QInputEvent which is a QEvent. Created attachment 69056 [details]
Tell the UI client when a key event is not handled
EWS++ Attachment 69056 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/Shared/win/NativeWebKeyboardEventWin.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/API/C/WKPage.h:166: Extra space between WKPageDidNotHandleKeyEventCallback and didNotHandleKeyEvent [whitespace/declaration] [3]
WebKit2/Shared/qt/NativeWebKeyboardEventQt.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/win/WebView.cpp:30: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 4 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 69056 [details] did not build on qt: Build output: http://queues.webkit.org/results/4042148 Created attachment 69058 [details]
Tell the UI client when a key event is not handled
Attachment 69058 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/Shared/win/NativeWebKeyboardEventWin.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/API/C/WKPage.h:166: Extra space between WKPageDidNotHandleKeyEventCallback and didNotHandleKeyEvent [whitespace/declaration] [3]
WebKit2/Shared/qt/NativeWebKeyboardEventQt.cpp:26: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4]
WebKit2/UIProcess/win/WebView.cpp:30: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 4 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 69058 [details] Tell the UI client when a key event is not handled View in context: https://bugs.webkit.org/attachment.cgi?id=69058&action=review > WebKit2/Shared/NativeWebKeyboardEvent.h:46 > + NativeWebKeyboardEvent(NSEvent *, NSView *); Why are you not left aligning the *'s? ObjC coding style? > WebKit2/Shared/NativeWebKeyboardEvent.h:58 > + const QEvent* nativeEvent() const { return &m_nativeEvent; } OK, seems fine. > WebKit2/UIProcess/API/C/WKPage.h:139 > +typedef const void* WKNativeEventPtr; OK, so in Qt we will just have to supply a QKeyEvent* > WebKit2/WebProcess/WebPage/WebPage.cpp:557 > + WebProcess::shared().connection()->send(WebPageProxyMessage::DidReceiveEvent, m_pageID, CoreIPC::In(static_cast<uint32_t>(touchEvent.type()), handled)); Didn't you make it easier to send these messages with your generator script? Comment on attachment 69058 [details] Tell the UI client when a key event is not handled View in context: https://bugs.webkit.org/attachment.cgi?id=69058&action=review >> WebKit2/Shared/NativeWebKeyboardEvent.h:46 >> + NativeWebKeyboardEvent(NSEvent *, NSView *); > > Why are you not left aligning the *'s? ObjC coding style? Yes. >> WebKit2/Shared/NativeWebKeyboardEvent.h:58 >> + const QEvent* nativeEvent() const { return &m_nativeEvent; } > > OK, seems fine. Whoops, I meant to make that return a const QKeyEvent*. >> WebKit2/UIProcess/API/C/WKPage.h:139 >> +typedef const void* WKNativeEventPtr; > > OK, so in Qt we will just have to supply a QKeyEvent* I did this for now until we find a better solution for Qt. Obviously it would be nice to have the following: typedef const QKeyEvent* WKNativeEventPtr; It seemed silly to hold up this patch while we figured out the details of how we're going to accomplish this. >> WebKit2/WebProcess/WebPage/WebPage.cpp:557 >> + WebProcess::shared().connection()->send(WebPageProxyMessage::DidReceiveEvent, m_pageID, CoreIPC::In(static_cast<uint32_t>(touchEvent.type()), handled)); > > Didn't you make it easier to send these messages with your generator script? Yes, but we haven't switched over everything to using the new system yet. Comment on attachment 69058 [details]
Tell the UI client when a key event is not handled
The patch seems fine to me, so r=me, but maybe you want Anders or Sam to have a look as well.
Created attachment 69065 [details]
Tell the UI client when a key event is not handled
Comment on attachment 69065 [details] Tell the UI client when a key event is not handled View in context: https://bugs.webkit.org/attachment.cgi?id=69065&action=review > WebKit2/UIProcess/WebPageProxy.cpp:711 > uint32_t type; > if (!arguments->decode(type)) > return; > - didReceiveEvent((WebEvent::Type)type); > + bool handled; > + if (!arguments->decode(handled)) > + return; > + didReceiveEvent((WebEvent::Type)type, handled); > break; > } We usually do this in one decode step using CoreIPC::Out(). (In reply to comment #19) > (From update of attachment 69065 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=69065&action=review > > > WebKit2/UIProcess/WebPageProxy.cpp:711 > > uint32_t type; > > if (!arguments->decode(type)) > > return; > > - didReceiveEvent((WebEvent::Type)type); > > + bool handled; > > + if (!arguments->decode(handled)) > > + return; > > + didReceiveEvent((WebEvent::Type)type, handled); > > break; > > } > > We usually do this in one decode step using CoreIPC::Out(). Fixed. Thanks! Committed r68538: <http://trac.webkit.org/changeset/68538> |