1094896 - [Dialer] Hash key still highlighted after calling MMI Code *#06#

Status: VERIFIED FIXED

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

Description

[Tamara Hills [:thills]]

STR:

1.  Enter *#06#
2.  IMEI should show up.
3.  Close and go back to dialer
4.  Expected: hash key is not highlighted.  Actual: hash key is highlighted.

Comment 2

[Gabriele Svelto [:gsvelto]]

It seems we're never receiving the touchend event for the hash key and since we're manually adding a CSS class to highlight it we don't remove it leaving the key highlighted. In my opinion we should switch this to using the CSS active property; this should also help in situations where the user is tapping quickly (we had a bug for that but I can't find it right now).

Comment 3

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH] Use a CSS selector for keypad highlight instead of a custom class

Hacked together a quick patch since I knew the code well already. This chops a significant portion of JS code in favor of a single-line change in the shared stylesheet.

IMHO this is the way to go but this does change the highlight behavior a bit. With the previous code if the user tapped a key, kept it pressed and moved his finger out of it, the button would remain highlighted until the user raised his finger. With my change as soon as the user moves out of the key then the highlight goes away.

I find this more consistent with how other UIs work. I've checked on both an Android and a Nokia S60 phone and on both the key highlight goes away as soon as the user drags his finger outside the key. I don't have an iOS device to test on though.

Comment 4

[Gabriele Svelto [:gsvelto]]

Attachment: [PULL REQUEST] Make sure we un-highlight the hash key when requesting the IMEI code(s)

Comment 5

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8524804 [details] [diff] [review]
[PATCH] Use a CSS selector for keypad highlight instead of a custom class

This patch breaks the fix added in bug 1037066. I think we should instead remove the `.active` class when displaying any results from USSD codes.

Comment 6

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH v2] Make sure we un-highlight the hash key when requesting the IMEI code(s)

Here's the updated patch. Still a one-liner but this time in the JS code. I've also updated the relevant unit-test to cover this particular issue.

Comment 7

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8524807 [details] [review]
[PULL REQUEST] Make sure we un-highlight the hash key when requesting the IMEI code(s)

The PR was updated too.

Comment 8

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8525333 [details] [diff] [review]
[PATCH v2] Make sure we un-highlight the hash key when requesting the IMEI code(s)

Review of attachment 8525333 [details] [diff] [review]:
-----------------------------------------------------------------

The test should be broken up into 2 tests.

Comment 9

[Gabriele Svelto [:gsvelto]]

I've split the tests, now waiting for try to become green before landing:

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=eecceb835a8d

Comment 10

[Gabriele Svelto [:gsvelto]]

I've re-run the tests and got a green run plus an intermittent which turned green after being re-triggered.

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=4b955a5780d7

Time to merge.

Comment 11

[Gabriele Svelto [:gsvelto]]

Pushed to gaia/master 9d2603f4b224237a365d89f44aa95f5a157b6db7

https://github.com/mozilla-b2g/gaia/commit/9d2603f4b224237a365d89f44aa95f5a157b6db7

Comment 12

[Shally Li（leave from Mozilla）]

This bug can be reproduced on Flame v2.1.
Attchments:reproduced_V2.1.mp4 and logcat_1623.txt.
Reproduce rate:5/8

Flame 2.1 Build:
Gaia-Rev        97873dca486abf4162a3345e71b375806937bdec
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/9faa165ac85d
Build-ID        20141211001204
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141211.034507
FW-Date         Thu Dec 11 03:45:22 EST 2014
Bootloader      L1TC00011880

Comment 13

[Shally Li（leave from Mozilla）]

Attachment: logcat_1623.txt

Comment 14

[Shally Li（leave from Mozilla）]

Attachment: reproduced_V2.1.MP4

Comment 15

[Shally Li（leave from Mozilla）]

Add Flame 2.2 Build:
Gaia-Rev        1a956437210d2b16988d2ddbf40c9a38d8474435
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/d92db71d4e67
Build-ID        20141211040208
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141211.074025
FW-Date         Thu Dec 11 07:40:36 EST 2014
Bootloader      L1TC00011880

Comment 16

[Mike Lien[:mlien]]

set blocking-b2g: v2.1? to see if this patch should be uplifted

Comment 17

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

NI to release management for triage.
Not a big patch, but 2.1 is already CC.

Comment 18

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: 
non-blocking since it's not regression and not breaking functionality. 
(pressing the # button again will un-highlight it.

Comment 19

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

minus, as it's already landed on master.

Comment 20

[Mike Lien[:mlien]]

refer to comment 18 and comment 19, set status-b2g-v2.1: wontfix