847851 - Event handler mozBluetooth.onadapteradded() in bluetooth.js may be replaced in connectivity.js

Status: RESOLVED FIXED

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

Description

[Eric Chou [:ericchou] [:echou]]

Gecko: 96d2212d71f0e90eb46454b7880e5d6723344ada
Gaia: fe8ee675312b90ccc881802d5bab9a50631ec83f
STR: Randomly turn on/off Bluetooth and kill Settings app. No 100% STR, however it seems to be more likely happened at the first time getting into Bluetooth page.

Sometimes when I tried to turn on/off Bluetooth in Settings, the switch may be disabled but not enabled. It's because that the event handler bluetooth.onadapteradded()/bluetooth.ondisabled() in Settings.js is replaced with gBluetooth.onadapteradded()/gBluetooth.ondisabled() in Connectivity.js for some reasons. 

Since there is a comment "these listeners are replaced when bluetooth.js is loaded" written in Connectivity.js, I was wondering if there's something wrong if Connectivity.js is loaded after bluetooth.js in this case.

Comment 2

[Evelyn Hung]

nominate it tef+ because it happens often after bug 841760, this may cause UI freeze then the user have to close and reopen Settings app.

Comment 3

[Evelyn Hung]

Attachment: change event hook to addEventListener

Comment 4

[Daniel Coloma:dcoloma]

I have reproduced this quite easily. Suggest we block on this one.

Comment 5

[David Flanagan [:djf]]

Bug 856229 might be a duplicate of this one. The report for that one includes this message in the logcat:

E/GeckoConsole(  474): [JavaScript Error: "pairList.index[deviceAddress] is undefined" {file: "app://settings.gaiamobile.org/js/bluetooth.js" line: 564}]

Evelyn, does that look related to you?

Comment 6

[Evelyn Hung]

I don't think it's a dup. I need time to investigate that issue more (also commented on bug 856229)

Comment 7

[Fabien Cazenave [:kaze]]

Comment on attachment 730029 [details]
change event hook to addEventListener

Looks good to me, merged into master:
https://github.com/mozilla-b2g/gaia/commit/bebae47fca4c9072f787e382a5cefafc4435d6ba

Comment 8

[Evelyn Hung]

Kaze, very thanks for your help.

Comment 9

[Jeni]

Can you please provide steps to verify this fix - as we will blackbox test from the UI?

Comment 10

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: 6a7682f8e723ddb20ddf9c81f5ce55ca2d57a01c

This commit does not apply to v1.0.1.  You could try resolving the issue with:

  cd gaia
  git checkout v1.0.1
  git cherry-pick 6a7682f8e723ddb20ddf9c81f5ce55ca2d57a01c
  <merge conflict>
  git add apps/settings/js/connectivity.js
  git commit

Comment 11

[Evelyn Hung]

Conflict resolved branch here, thanks.

https://github.com/evelynhung/gaia/tree/uplift-847851-v1.0.1

Comment 12

[Evelyn Hung]

(In reply to Jeni from comment #9)
> Can you please provide steps to verify this fix - as we will blackbox test
> from the UI?

Try this:
1. launch settings app (which haven't been launched before)
2. when seeing bluetooth label, click it as soon as possible.
3. turn bluetooth on

Expected Result:
- the switch toggle will be disabled for a while and then be enabled when bluetooth interface is ready.

If the issue didn't fixed, you will see the switch toggle is disabled forever.

Comment 13

[Daniel Coloma:dcoloma]

John, can you try the uplift now?

Comment 16

[Jeni]

Verified fixed on 

Unagi Build ID: 20130410070209
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/423f7851bdb5
Gaia: c614b3f3c956dc1e1adf93cf4cf41511ce75de80

and

Verified fixed on Master Build

Updated to Unagi Build ID: 20130410065939
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/mozilla-central/rev/ee5ca214e87c
Gaia: f6587418cc5fd8dfe885539ae1a798473890c7f5

Comment 17

[Evelyn Hung]

I merged into v1.0.1 by myself.
https://github.com/mozilla-b2g/gaia/commit/e7f13fb1577dd613bea4a7908da3920089eb2ad1

Comment 19

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Evelyn Hung [:evelyn] from comment #17)
> I merged into v1.0.1 by myself.
> https://github.com/mozilla-b2g/gaia/commit/
> e7f13fb1577dd613bea4a7908da3920089eb2ad1

This commit was unfortunately not resolved correctly maybe due to some git quirks. It has been backed out here.

https://github.com/mozilla-b2g/gaia/commit/896727c3635968b2deac23d301153fb82d1aa15c

Comment 20

[Evelyn Hung]

> This commit was unfortunately not resolved correctly maybe due to some git
> quirks. It has been backed out here.
> 
> https://github.com/mozilla-b2g/gaia/commit/
> 896727c3635968b2deac23d301153fb82d1aa15c
I was unexpectedly including one line from another patch which won't be uplifted.
Resolved it correctly and do the uplift again. Sorry about it.

https://github.com/mozilla-b2g/gaia/commit/e1d9f92df539460b9483c892590bb78940307c61