955925 - [Messages] Threads cannot be opened when retrieving the corresponding contact fails

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

+++ This bug was initially created as a clone of Bug #907740 +++

Bug 907740 introduced an issue whereas if navigator.mozContacts.find() fails and invokes the onerror() path then findByPhoneNumber() will trigger an error as it doesn't verify if the |results| argument is defined before accessing it.

STR, start with a freshly flashed phone:

1. Install thje light reference workload
   make reference-workload-light APP=sms
2. Open the SMS app
3. Try tapping on the large SMS thread, the thread won't open and an error will be logged in the logcat

Comment 1

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH] Properly handle the result of a failed contact retrieval operation

Comment 2

[Gabriele Svelto [:gsvelto]]

Attachment: [PULL REQUEST] Properly handle the result of a failed contact retrieval operation

Comment 3

[Rick Waldron [:rwaldron]]

I suspect this is actually a duplicate of bug 952781, since thread rendering with or without a stored contact worked just fine prior to the patch that caused that bug.

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 8355080 [details] [diff] [review]
[PATCH] Properly handle the result of a failed contact retrieval operation

r=me

can you try to do a unit test before landing? Should be reasonnably easy, I'm sure we already have a test with an onerror call.

Comment 8

[Gabriele Svelto [:gsvelto]]

Attachment: [PATCH v2] Properly handle the result of a failed contact retrieval operation

Now with unit-test attached :)

(the way I trigger the error is not exactly pretty but I could not find a cleaner way with the existing mock)

Comment 9

[Rick Waldron [:rwaldron]]

Julien, this looks good to me

Comment 10

[Rick Waldron [:rwaldron]]

Comment on attachment 8356113 [details] [diff] [review]
[PATCH v2] Properly handle the result of a failed contact retrieval operation

Thanks for addressing this issue!

r=me

Comment 11

[Gabriele Svelto [:gsvelto]]

Thanks for the review, Travis is green:

https://travis-ci.org/mozilla-b2g/gaia/builds/16468626

Pushed to gaia/master b0442b2867a78f30e26cee23aba4de3edbb9e86a

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b62a81ca06f8

Comment 13

[Julien Wajsberg [:julienw]]

My r+ was already given in comment 7 ;)