RESOLVED FIXED 83340
[BlackBerry] Enable vibration for the BlackBerry port
https://bugs.webkit.org/show_bug.cgi?id=83340
Summary [BlackBerry] Enable vibration for the BlackBerry port
George Staikos
Reported 2012-04-05 18:04:22 PDT
Created attachment 135950 [details] Patch to enable vibration Enable the vibration API for BlackBerry.
Attachments
Patch to enable vibration (8.04 KB, patch)
2012-04-05 18:04 PDT, George Staikos
no flags
Patch to enable vibration (8.20 KB, patch)
2012-04-05 18:11 PDT, George Staikos
no flags
Turn on vibration by default now (1.25 KB, patch)
2012-04-06 09:55 PDT, George Staikos
no flags
WebKit Review Bot
Comment 1 2012-04-05 18:08:14 PDT
Attachment 135950 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebKit/ChangeLog', u'..." exit_code: 1 ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebKit/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebKit/blackberry/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebKit/blackberry/WebCoreSupport/VibrationClientBlackBerry.cpp:20: You should add a blank line after implementation file's own header. [build/include_order] [4] Source/WebKit/blackberry/WebCoreSupport/VibrationClientBlackBerry.h:29: Missing space inside { }. [whitespace/braces] [5] Total errors found: 5 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
George Staikos
Comment 2 2012-04-05 18:11:34 PDT
Created attachment 135954 [details] Patch to enable vibration
Rob Buis
Comment 3 2012-04-05 18:16:31 PDT
Comment on attachment 135954 [details] Patch to enable vibration View in context: https://bugs.webkit.org/attachment.cgi?id=135954&action=review Looks good. > Source/WebKit/blackberry/WebCoreSupport/VibrationClientBlackBerry.cpp:30 > +} May want to keep this in the header inline for now. > Source/WebKit/blackberry/WebCoreSupport/VibrationClientBlackBerry.cpp:38 > +{ Ditto. > Source/cmake/OptionsBlackBerry.cmake:178 > +WEBKIT_FEATURE(ENABLE_VIBRATION "Enable vibration support" DEFAULT ON) Any change needed in build-webkit? cmakeconfig.cmake? Usually they go hand-in-hand.
George Staikos
Comment 4 2012-04-05 18:17:53 PDT
(In reply to comment #3) > (From update of attachment 135954 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135954&action=review > > Looks good. > > > Source/WebKit/blackberry/WebCoreSupport/VibrationClientBlackBerry.cpp:30 > > +} > > May want to keep this in the header inline for now. > > > Source/WebKit/blackberry/WebCoreSupport/VibrationClientBlackBerry.cpp:38 > > +{ > > Ditto. > > > Source/cmake/OptionsBlackBerry.cmake:178 > > +WEBKIT_FEATURE(ENABLE_VIBRATION "Enable vibration support" DEFAULT ON) > > Any change needed in build-webkit? cmakeconfig.cmake? Usually they go hand-in-hand. build-webkit change will go in separately later. cmakeconfig change is already there. Actually it's very annoying that we need to change build-webkit. We should be able to turn things on by default if they're not specified there.
WebKit Review Bot
Comment 5 2012-04-05 19:27:50 PDT
Comment on attachment 135954 [details] Patch to enable vibration Clearing flags on attachment: 135954 Committed r113404: <http://trac.webkit.org/changeset/113404>
WebKit Review Bot
Comment 6 2012-04-05 19:27:55 PDT
All reviewed patches have been landed. Closing bug.
George Staikos
Comment 7 2012-04-06 09:55:22 PDT
Created attachment 136037 [details] Turn on vibration by default now
George Staikos
Comment 8 2012-04-06 09:55:42 PDT
Reopen pending the build-webkit patch
Rob Buis
Comment 9 2012-04-06 09:56:13 PDT
Comment on attachment 136037 [details] Turn on vibration by default now LGTM.
WebKit Review Bot
Comment 10 2012-04-06 13:13:00 PDT
Comment on attachment 136037 [details] Turn on vibration by default now Clearing flags on attachment: 136037 Committed r113489: <http://trac.webkit.org/changeset/113489>
WebKit Review Bot
Comment 11 2012-04-06 13:13:05 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.