Bug 101655 - [BlackBerry] Adding a sound to touch events on anchor elements
Summary: [BlackBerry] Adding a sound to touch events on anchor elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-08 15:23 PST by otcheung
Modified: 2012-11-09 14:34 PST (History)
4 users (show)

See Also:


Attachments
Patch (4.21 KB, patch)
2012-11-08 15:42 PST, otcheung
no flags Details | Formatted Diff | Diff
Patch (4.34 KB, patch)
2012-11-09 09:18 PST, otcheung
no flags Details | Formatted Diff | Diff
Patch (4.35 KB, patch)
2012-11-09 11:14 PST, otcheung
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description otcheung 2012-11-08 15:23:43 PST
Adding an input click to touch events on anchor elements
Comment 1 otcheung 2012-11-08 15:42:19 PST
Created attachment 173138 [details]
Patch
Comment 2 Antonio Gomes 2012-11-08 18:35:32 PST
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 3 otcheung 2012-11-09 09:00:03 PST
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
Comment 4 otcheung 2012-11-09 09:18:29 PST
Created attachment 173325 [details]
Patch
Comment 5 Antonio Gomes 2012-11-09 11:03:17 PST
(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.
Comment 6 Antonio Gomes 2012-11-09 11:03:57 PST
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.
Comment 7 otcheung 2012-11-09 11:14:50 PST
Created attachment 173339 [details]
Patch
Comment 8 WebKit Review Bot 2012-11-09 14:03:59 PST
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
Comment 9 Rob Buis 2012-11-09 14:09:56 PST
Comment on attachment 173339 [details]
Patch

Retrying
Comment 10 WebKit Review Bot 2012-11-09 14:34:04 PST
Comment on attachment 173339 [details]
Patch

Clearing flags on attachment: 173339

Committed r134117: <http://trac.webkit.org/changeset/134117>
Comment 11 WebKit Review Bot 2012-11-09 14:34:07 PST
All reviewed patches have been landed.  Closing bug.