1152675 - [Bluetooth][APIv2] JS error while goes into Settings::Bluetooth panel

Status: RESOLVED FIXED

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

Description

[[e-mail], [e-mail]]

JS error while goes into Settings::Bluetooth panel.

LOG:

W/Settings(19694): [JavaScript Error: "TypeError: this is undefined" {file: "app://settings.gaiamobile.org/js/panels/bluetooth/panel.js" line: 340}]

Comment 1

[[e-mail], [e-mail]]

This is a regression from Bug 1144532. It was missed to bind this while regedit _updateVisibilityTimer() for observable object.

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

Comment 3

[[e-mail], [e-mail]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

Arthur, I feel ashamed that I missed to bind 'this' for _updateVisibilityTimer(). Will need your review here again. Thanks.

Comment 4

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

oh... too bad. I did not catch this when reviewing.

To avoid this kind of error happens again, I would suggest to have a "bound" version of the handlers. ex:

this._boundUpdateXXX = this._updateXXX.bind(this);

And in `hide`, let's remove the handlers explicitly.

Comment 5

[[e-mail], [e-mail]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

Arthur, Nice catch here for the suggestion. I have updated the patch with the way. That would be more clear than before.

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

Let's bind "this" to ALL functions before using them as an observer. That avoids potential issues while adding reference to "this" in the future but forget to bind the function.

Comment 7

[[e-mail], [e-mail]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

Arthur, I have updated the patch for all callback functions which observe Bluetooth context. And we don't need to un-observe each callback functions since we have un-observed these properties in 'onBeforeHide' stage. Need your review. Thanks.

Comment 8

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

Sorry I had to cancel the request again but it should be easy to fix.

BluetoothContext is a common module, in the sense that other panels may also observe the property change. Doing `BtContext.unobserve('state');` removes ALL handlers even they are registered by other panels. Although currently we have no such use case but I would suggest to make it robust by removing the handlers explicitly.

Comment 9

[[e-mail], [e-mail]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

Nice catch in this view point. We have to use 'unobserve' carefully. Thanks for the reminder:)

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8590118 [details] [review]
[gaia] ian-liu:bluetooth/bug1152675_btv2_JS_error_while_goes_into_Bluetooth_panel > mozilla-b2g:master

r=me, thanks!

Comment 11

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/cd43da30b1c3a0ee00e9d6f6f7d744e3b852fd62