Closed Bug 1219249 Opened 9 years ago Closed 9 years ago

[Dialer] Spamming keypad with multi-tap inputs during a call can cause the device to become unresponsive.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.5 fixed, b2g-master fixed)

RESOLVED FIXED
Tracking Status
b2g-v2.5 --- fixed
b2g-master --- fixed

People

(Reporter: gsvelto, Assigned: gsvelto)

References

()

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1178451 +++

This is an old issue that leads to some pretty dire consequences as seen in bug 1178451. The solution is to disable multiple taps to the keypad as they make no sense even logically speaking (e.g. it's impossible to send more than one dial-tone at the time, etc...).
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Comment on attachment 8680302 [details] [review]
[gaia] gabrielesvelto:bug-1219249-disable-keypad-multitap > mozilla-b2g:master

This patch prevents the user from tapping more than one key at the same time in the keypad thus preventing all the associated races through the code (which assumed only one key could be tapped at the same time but didn't enforce it). 

Additionally I've fixed another issue that nobody every noticed but had always been there. When tapping the # key we should have acted immediately if the typed code was the IMEI request code (*#06#) or a speed-dial number (N[N][N]#), i.e. we should have acted on the code during the 'touchstart' event. As it turns out we weren't. We were executing the 'touchstart' handler that would update the |_phoneNumber| field and then on the following 'touchmove' event (or 'touchend' if no moves happened) we acted. This indirectly caused weird bugs (e.g. bug 1094896) which were fixed by duplicating code from the 'touchstart' handler in other places. All that stuff's been removed now and I've factored out the function used to handle speed dialing numbers to make the intent clearer and the handler a little leaner.

The patch doesn't have added unit-tests for now, I'll add them tomorrow.
Attachment #8680302 - Flags: review?(thills)
I've added an additional unit-test to cover this and streamlined the others with simpler mocks for the touchstart/touchend events.
Comment on attachment 8680302 [details] [review]
[gaia] gabrielesvelto:bug-1219249-disable-keypad-multitap > mozilla-b2g:master

Hi Gabriele,

It looks good.  I would just file a follow up for the emergency keypad.js.

Thanks,

-tamara
Attachment #8680302 - Flags: review?(thills) → review+
Merged to gaia/master 10160e1047d2a58ad51cec79f6e253f7cd07e858

https://github.com/mozilla-b2g/gaia/commit/10160e1047d2a58ad51cec79f6e253f7cd07e858

I'll open a follow-up for the emergency keypad; I'd also like this to be uplifted to 2.5 so I'll be setting the appropriate flags.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 8680302 [details] [review]
[gaia] gabrielesvelto:bug-1219249-disable-keypad-multitap > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Long standing issue with the code
[User impact] if declined: The user can tap multiple keys at the same time leading to inconsistent results (wrong dialtone, wrong number typed, in extreme cases the UI locks up, see also bug 1100274 and bug 1100230)
[Testing completed]: Covered with unit-tests and smoke-tested most of the scenarios on a device
[Risk to taking this patch] (and alternatives if risky): This could introduce problems when typing in the keypad
[String changes made]: None
Attachment #8680302 - Flags: approval-gaia-v2.5?
Comment on attachment 8680302 [details] [review]
[gaia] gabrielesvelto:bug-1219249-disable-keypad-multitap > mozilla-b2g:master

Tomcat - Can you please check this is in 2.5 with the 11/4 update if not land it. 

Thanks
Attachment #8680302 - Flags: approval-gaia-v2.5? → approval-gaia-v2.5+
(In reply to Mahendra Potharaju [:mahe] from comment #7)
> Comment on attachment 8680302 [details] [review]
> [gaia] gabrielesvelto:bug-1219249-disable-keypad-multitap >
> mozilla-b2g:master
> 
> Tomcat - Can you please check this is in 2.5 with the 11/4 update if not
> land it. 
> 
> Thanks

Verified that its in 2.5. Thanks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: