[NFC] Incorrect BT confirm dialog when NFC headset detected while BT initial ON

VERIFIED FIXED in 2.2 S5 (6feb)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: ashiue, Assigned: gasolin)

Tracking

({regression})

unspecified
2.2 S5 (6feb)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

Details

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
Gaia-Rev        237008137f6d72b9cad25ff4faff14ff2c40ac63
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/be24dd482a83
Build-ID        20150122162504
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150122.194331
FW-Date         Thu Jan 22 19:43:41 EST 2015
Bootloader      L1TC000118D0

A phone with NFC, and a NFC-capable bluetooth earphone

STR: 
1. Enable NFC and Bluetooth in Settings
2. Tap the phone with NFC-capable bluetooth earphone
3. Check the confirmation dialog

Expect result: 
Show "Are you sure you want to connect to XXXX?"

Actual result:
Show "Turn on Bluetooth and connect to XXXX?"
(Reporter)

Comment 1

4 years ago
Please refer page 6.
(Reporter)

Updated

4 years ago
blocking-b2g: --- → 2.2?
QA Whiteboard: [COM=NFC]
(Reporter)

Updated

4 years ago
Summary: [NFC] Incorrect BT confirm dialog when NFC headset detected while BT initial off → [NFC] Incorrect BT confirm dialog when NFC headset detected while BT initial ON
triage: regression
blocking-b2g: 2.2? → 2.2+
Flags: needinfo?(iliu)
Keywords: regression
Flags: needinfo?(shuang)
I will check this bug first. Thanks!
Flags: needinfo?(shuang)
Flags: needinfo?(iliu)
Hmm...I cannot reproduce this bug. I did not see any extra pairing confirmation log.
This would be a gaia bug, and this issue is also reproducible on the master.

|Bluetooth| module doesn't have |name| attribute [1], so |Service| doesn't have a correct mapping after |registerState()| [2][3], and |enabled| in |ncsd_setMessage()| is |undefined| as a result.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth.js#L5
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth.js#L142
[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/service.js#L180
[4] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/system_nfc_connect_dialog.js#L21
Probably it is needed to use |BaseModule| [1] to implement |Bluetooth| module like other modules as well.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/base_module.js#L336
Hi Ian, would you mind having a look on it?
Flags: needinfo?(iliu)
Fred, could you please take a look the issue? Per comment 5, [2][3] was changed for decoupling `bluetooth`, `nfc_handler_manager` before. Looks like some regression might be happened here.
Flags: needinfo?(iliu) → needinfo?(flin)
(Assignee)

Comment 9

4 years ago
Thanks bruce for tracing the issue. add `name = 'Bluetooth',` below https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth.js#L5 might work.

I'll get the NFC headset to test.
(Assignee)

Updated

4 years ago
Flags: needinfo?(flin)
(Assignee)

Comment 10

4 years ago
sorry I mean `name: 'Bluetooth',`
(Assignee)

Comment 11

4 years ago
add name attribute to make Service.registerState works correctly.

The related messages shows correctly with this patch (thanks Alison's help)
Attachment #8556317 - Flags: review?(alive)
(Assignee)

Updated

4 years ago
Assignee: nobody → gasolin
Attachment #8556317 - Flags: review?(alive) → review+
(Assignee)

Updated

4 years ago
Component: NFC → Gaia::System
Keywords: checkin-needed
(Assignee)

Comment 12

4 years ago
Comment on attachment 8556317 [details] [review]
pull request redirect to github

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): regress of Bug 1092893
[User impact] if declined: nfc headset with wrong message
[Testing completed]: manual test passed
[Risk to taking this patch] (and alternatives if risky): none
[String changes made]: none
Attachment #8556317 - Flags: approval-gaia-v2.2?
(Assignee)

Comment 13

4 years ago
this issue is also fixed by Bug 1092894 in master.

But the patch is needed to uplift to 2.2
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
No need for checkin-needed on uplifts. It'll show up on the right radar once it's approved.
Target Milestone: --- → 2.2 S5 (6feb)
That said, can you please post a 2.2-specific PR for this (or update the original one)? I don't know of any easy way to redirect an existing PR to a different branch than it was originally intended for.
Flags: needinfo?(gasolin)
Keywords: checkin-needed
Attachment #8556317 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
(Reporter)

Comment 17

4 years ago
Verified on
[2.2]
Gaia-Rev        21cce750c095a3f815275fe5439fa9dbfe3dfc6b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/41ccc5328740
Build-ID        20150209162506
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150209.194557
FW-Date         Mon Feb  9 19:46:08 EST 2015
Bootloader      L1TC000118D0

[3.0]
Gaia-Rev        0cf517083f7eb5fc269e1236edba50534f65e3cd
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/2cb22c058add
Build-ID        20150209160204
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150209.193059
FW-Date         Mon Feb  9 19:31:10 EST 2015
Bootloader      L1TC000118D0
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.