Bug 101655 – [BlackBerry] Adding a sound to touch events on anchor elements

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

Adding an input click to touch events on anchor elements

Created attachment 173138 [details]
Patch

[Antonio Gomes]

Comment on attachment 173138 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173138&action=review

why instead of the client calling this Api, why not make TouchEventHandler to do it on single touch/mouse up?

> Source/WebKit/blackberry/Api/WebPage.h:159
> +    void playSoundOnAnchorElementTouchEvents();

bad naming imo

something around "play sound if anchor is the target" would read better

> Source/WebKit/blackberry/WebKitSupport/TouchEventHandler.cpp:187
> +void TouchEventHandler::playSoundOnAnchorElementTouchEvents()

const?

Comment on attachment 173138 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173138&action=review

Hi Antonio! We tried playing the audio when we receive a mouseup event, but the problem is the delay we get from the timer in recognizing tap sequence is preventing instant feedback. That's why we need to trigger the sound manually when we get a valid click event in libwebview.

>> Source/WebKit/blackberry/Api/WebPage.h:159
>> +    void playSoundOnAnchorElementTouchEvents();
> 
> bad naming imo
> 
> something around "play sound if anchor is the target" would read better

agreed. playSoundIfAnchorIsTarget sounds better

Created attachment 173325 [details]
Patch

[Antonio Gomes]

(In reply to comment #3)
> (From update of attachment 173138 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173138&action=review
> 
> Hi Antonio! We tried playing the audio when we receive a mouseup event, but the problem is the delay we get from the timer in recognizing tap sequence is preventing instant feedback. That's why we need to trigger the sound manually when we get a valid click event in libwebview.

Fair reason.

[Antonio Gomes]

Comment on attachment 173325 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173325&action=review

> Source/WebKit/blackberry/Api/WebPage.cpp:4168
> +void WebPage::playSoundIfAnchorIsTarget()

const here too, please.

Created attachment 173339 [details]
Patch

[WebKit Review Bot]

Comment on attachment 173339 [details]
Patch

Rejecting attachment 173339 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
'Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 9
Updating OpenSource
From http://git.chromium.org/external/Webkit
 + 9031965...c362e10 master     -> origin/master  (forced update)
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 134105 = 9031965ca29f13cb1f737dc551fab02709cd71a8
last_rev is higher!: 134105 >= 134102 at /usr/lib/git-core/git-svn line 1523

Died at Tools/Scripts/update-webkit line 154.

Full output: http://queues.webkit.org/results/14788267

[Rob Buis]

Comment on attachment 173339 [details]
Patch

Retrying

[WebKit Review Bot]

Comment on attachment 173339 [details]
Patch

Clearing flags on attachment: 173339

Committed r134117: <http://trac.webkit.org/changeset/134117>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.