829406 - When in call the keypad should react on click event and not on mousedown

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

While calling my voice mails on my phone to listen for them I end up displaying the dialer keyboard to enter dtmf tones in order to delete/save/... messages. At some point I put the finger on the keypad and try to move it away because I changed my mind. But my hit was still registered since the keypad result on keydown and the command was taken by my voice mail.

I end up doing something inoffensive but I could have delete message I want to keep... Nominating for the user data loss.

Comment 1

[David Scravaglieri [:scravag]]

Just be more careful, not V1

Comment 2

[Dylan Oliver [:doliver]]

How does this reconcile with the requirement that we need to support long tones, i.e. we need to sound the DTMF tone for as long as you press the button.

Comment 3

[Germán Toro del Valle (:gtorodelvalle)]

I just checked that in Android 4.2.1 the behavior is kind of the same. In fact, I would say it is even worse: the tone keeps on sending until you take your finger off the screen, no matter if you have moved your finger around to several adjacent keys before that :-O

Apart from this, I totally agree with :doliver ;-) It seems that in Brazil, at least, there is the possibility to send variable duration tones ;-999

Comment 4

[Julien Wajsberg [:julienw]]

be careful to Bug 815439, we don't want to screw up long tones.
(or do we ?)

Comment 5

[Julien Wajsberg [:julienw]]

Michal stole that bug.

Comment 6

[Michał Budzyński (:michalbe)]

Attachment: patch

Comment 7

[Etienne Segonzac (:etienne)]

Comment on attachment 760222 [details]
patch

Like Julien said in Comment 4, we shouldn't regress bug 815439.

We can try to check if the DTMF long press is negotiable, but without a clear GO from product that this won't cause any certification issue we can't drop it.

Comment 8

[Julien Wajsberg [:julienw]]

The plan Michal and I discussed about was :

* on mousedown/touchstart, play a sound that sounds like a DTMF sound, but don't send it over the network. Instead, keep the moment of that mousedown
* if there were no mouseleave outside of the start key, then, on mouseup (in the same key), stop the sound, and send the DTMF sound with the same length. That DTMF sound should not be heard by the user
* if there was a mouseleave outside of the start key, then cancel the sending of that tone.

Etienne, what do you think of this plan ?
Obviously the patch was not at all like this :)

Comment 9

[Etienne Segonzac (:etienne)]

(In reply to Julien Wajsberg [:julienw] from comment #8)
> The plan Michal and I discussed about was :
> 
> * on mousedown/touchstart, play a sound that sounds like a DTMF sound, but
> don't send it over the network. Instead, keep the moment of that mousedown
> * if there were no mouseleave outside of the start key, then, on mouseup (in
> the same key), stop the sound, and send the DTMF sound with the same length.
> That DTMF sound should not be heard by the user
> * if there was a mouseleave outside of the start key, then cancel the
> sending of that tone.
> 
> Etienne, what do you think of this plan ?
> Obviously the patch was not at all like this :)

sounds really good :)

Comment 10

[Jakub Siemiatkowski (:sasklacz)]

Attachment: patch

Comment 11

[Jakub Siemiatkowski (:sasklacz)]

Patch created as discussed with :michalbe

Comment 12

[Michał Budzyński (:michalbe)]

Thanks owcarnia! Now, could you please squash all the commits into one and change the title to something meaningful?

Comment 13

[Jakub Siemiatkowski (:sasklacz)]

Attachment: patch with squashed commits

Comment 14

[Etienne Segonzac (:etienne)]

Comment on attachment 765121 [details] [diff] [review]
patch with squashed commits

Hey,

looks like there is a typo :) (see github)
(please ask for r? again once the PR is updated, thanks!)

Comment 15

[Etienne Segonzac (:etienne)]

Comment on attachment 765121 [details] [diff] [review]
patch with squashed commits

Almost there!

but we don't want to send the DTMF when we're not on a call (see github)
and we need to clear the timeout properly in one case.

Cheers!

Comment 16

[Etienne Segonzac (:etienne)]

Comment on attachment 765121 [details] [diff] [review]
patch with squashed commits

Sorry, so close...

Your forgot the |clearTimeout|...

Comment 17

[Etienne Segonzac (:etienne)]

Comment on attachment 765121 [details] [diff] [review]
patch with squashed commits

cool, r=me with the 2 lasts comments on github addressed.

Ping me when the travis build is green and I'll check the code in.

Thanks for the effort, it takes some time to get it right :)

Comment 18

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/8e5d58110ed0ae309bc4e4683e68414ff9dab2e2

Congrats on you first gaia patch! :)