Closed Bug 829406 Opened 7 years ago Closed 7 years ago

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

Categories

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

x86_64
Linux
defect
Not set

Tracking

(blocking-basecamp:-, b2g18+)

RESOLVED FIXED
blocking-basecamp -
Tracking Status
b2g18 + ---

People

(Reporter: vingtetun, Assigned: sasklacz)

References

Details

(Whiteboard: u=fx-os-user c=scravag-sprint p=1)

Attachments

(2 files, 1 obsolete file)

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.
Just be more careful, not V1
blocking-basecamp: ? → -
tracking-b2g18: --- → +
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.
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
Whiteboard: u=fx-os-user c=scravag-sprint-may-20-31 p=1
Whiteboard: u=fx-os-user c=scravag-sprint-may-20-31 p=1 → u=fx-os-user c=scravag-sprint p=1
Assignee: nobody → felash
be careful to Bug 815439, we don't want to screw up long tones.
(or do we ?)
Michal stole that bug.
Assignee: felash → mbudzynski
Attached file patch
Attachment #760222 - Flags: review?(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.
Attachment #760222 - Flags: review?(etienne) → review-
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 :)
Flags: needinfo?(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 :)
Flags: needinfo?(etienne)
Attached patch patch (obsolete) — Splinter Review
Attachment #765074 - Flags: review?(etienne)
Patch created as discussed with :michalbe
Thanks owcarnia! Now, could you please squash all the commits into one and change the title to something meaningful?
Attachment #765121 - Flags: review?(etienne)
Attachment #765074 - Attachment is obsolete: true
Attachment #765074 - Flags: review?(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!)
Attachment #765121 - Flags: review?(etienne)
Attachment #765121 - Flags: review?(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!
Attachment #765121 - Flags: review?(etienne)
Assignee: mbudzynski → owcarnia
Attachment #765121 - Flags: review?(etienne)
Comment on attachment 765121 [details] [diff] [review]
patch with squashed commits

Sorry, so close...

Your forgot the |clearTimeout|...
Attachment #765121 - Flags: review?(etienne)
Attachment #765121 - Flags: review?(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 :)
Attachment #765121 - Flags: review?(etienne) → review+
https://github.com/mozilla-b2g/gaia/commit/8e5d58110ed0ae309bc4e4683e68414ff9dab2e2

Congrats on you first gaia patch! :)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.