981327 - [JavaScript Error: 'TypeError: item is undefined' {file: app://communications.gaiamobile.org/shared/js/binary_search.js line: 26}]

Status: RESOLVED INVALID

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

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

I don't have Facebook contacts on my phone and this error keeps popping up when I'm typing a phone number in the dialer keypad. It seems like treeTel is an |object| instead of an |array| and the binarySearch utils complains.

I can't find in the codebase where treeTel is define as an object, so I wonder if that's a leftover from an old facebook datastore ?

In all cases the above patch prevent the errors to be generated while I believe it does not break anything since my understanding is that the index is generated just to make the code not generate errors.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug981327.patch

Comment 2

[Jose Manuel Cantera]

Comment on attachment 8388106 [details] [diff] [review]
bug981327.patch

the proposed patch will always reset the index so it is not the correct solution. Attached is proposed a different solution. Vivien, please check that my proposal fixes the problem for you

Comment 3

[Jose Manuel Cantera]

Attachment: 17020.html

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8388428 [details]
17020.html

I didn't tried it locally but looking at the code I'm pretty sure it solve the issue for me, since it is really related to treeTel beeing an object.

Comment 5

[Jose Manuel Cantera]

landed in master:

https://github.com/mozilla-b2g/gaia/commit/2ac4603e3269c5d410869a76c13a25bdb6a79241

Comment 6

[Michael Henretty [:mikehenrty][:mhenretty]]

Sorry guys, I had to back this out for causing regressing bug 982150. Not sure why the SMS tests didn't catch this in the first place, but would be good to add a test that would catch it before re-landing.

https://github.com/mozilla-b2g/gaia/commit/2dd9e509aeec0cb01bee17f42494b0a501128792

Comment 7

[Julien Wajsberg [:julienw]]

The main issue here is not really the SMS tests, but it's that this piece of code was r+ without tests ;) And that a perfectly expected situation like "result=null" was not tested in existing tests either.

Please add a test for at least this situation before relanding.

Comment 8

[Doug Sherk (:drs) (inactive)]

Is this still an issue? I haven't seen this recently.

Comment 9

[Doug Sherk (:drs) (inactive)]

Assuming that this is fixed given the lack of response and recent reports.