[Qt] Add support for Maemo zoom keys in QtLauncher
Created attachment 47399 [details] Patch
Attachment 47399 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKitTools/QtLauncher/main.cpp:106: Alphabetical sorting problem. [build/include_order] [4] WebKitTools/QtLauncher/main.cpp:107: Alphabetical sorting problem. [build/include_order] [4] WebKitTools/QtLauncher/main.cpp:213: QEvent_KeyPress is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 3 If any of these errors are false positives, please file a bug against check-webkit-style.
I am not too familiar with Maemo or its supported devices, but is there a reason why you chose the keys F7 and F8 for zooming in and out, respectively, as opposed to conforming to the + and - keys used currently by QtLauncher, Safari, and Firefox? Also, wouldn't this change break current functionality: - } else if (event->type() == QEvent::KeyPress + } else if (event->type() == QEvent_KeyPress You seem to point this out by your comment: // MAEMO: The X11 headers macro "KeyPress" which breaks "QEvent::KeyPress"... Moreover, can you elaborate what you implying by the "..."?
(In reply to comment #3) > I am not too familiar with Maemo or its supported devices, but is there a > reason why you chose the keys F7 and F8 for zooming in and out, respectively, > as opposed to conforming to the + and - keys used currently by QtLauncher, > Safari, and Firefox? the zoom dedicated HW keys are mapped to f7 and f8 on maemo
Comment on attachment 47399 [details] Patch > + > +#ifdef Q_WS_MAEMO_5 > + ~MainWindow() > + { > + grabZoomKeys(false); > } > +#endif I suggest to move the #ifdefs into the function body. (as well as in keyPressEvent) > #if QT_VERSION >= QT_VERSION_CHECK(4, 6, 0) > void sendTouchEvent() > @@ -193,6 +209,9 @@ public: > > bool eventFilter(QObject* obj, QEvent* event) > { > + // MAEMO: The X11 headers macro "KeyPress" which breaks "QEvent::KeyPress"... > + const int QEvent_KeyPress = 6; A better way to fix this is to #undef KeyPress after including the X11 headers. The rest looks good to me :)
Created attachment 47512 [details] Patch Same patch with raised issues addressed. For info on the F7/F8 keys, see http://qt.nokia.com/doc/qt-maemo-4.6/maemo5-zoom.html
Comment on attachment 47512 [details] Patch Rejecting patch 47512 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Simon Hausmann', '--force']" exit_code: 1 patching file WebKitTools/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebKitTools/QtLauncher/main.cpp Hunk #1 FAILED at 50. Hunk #2 succeeded at 118 (offset -55 lines). Hunk #3 succeeded at 401 (offset -55 lines). 1 out of 3 hunks FAILED -- saving rejects to file WebKitTools/QtLauncher/main.cpp.rej Full output: http://webkit-commit-queue.appspot.com/results/215236
Created attachment 47693 [details] Same patch, updated to merge
Comment on attachment 47693 [details] Same patch, updated to merge > +#ifdef Q_WS_MAEMO_5 > + grabZoomKeys(false); > +#endif I would prefer having the ifdef tests inside grabZoomKeys instead. > if (!winId()) > qWarning("can't grab keys unless we have a window id"); Why no return after this?
Created attachment 47705 [details] Same patch, issues addressed Thanks for the comments, Kenneth!
Comment on attachment 47705 [details] Same patch, issues addressed Clearing flags on attachment: 47705 Committed r54058: <http://trac.webkit.org/changeset/54058>
All reviewed patches have been landed. Closing bug.