[SMS][MMS] contacts without a phone number entry will break the contacts lookup in the Sms app

VERIFIED FIXED

Status

VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: rwaldron, Assigned: rwaldron)

Tracking

unspecified
x86
Mac OS X

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 verified, b2g18-v1.0.1 verified)

Details

(Whiteboard: [status: needs uplift])

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Seems serious.
Summary: [SMS][MMS] contacts without a phone number entry will break the SMS app → [SMS][MMS] contacts without a phone number entry will break the contacts lookup in the Sms app
ok, Bugzilla swallowed my description. And your first comment was made a description...

Let me write it again...
STR:
* Have 3 contacts: Julian, Julie and Juliette. Julian and Juliette only have a phone number, but not Julie.
* From the Sms App, look up "Juli"

Expected:
* we get all the contacts that have a phone number, that is Julian and Juliette.

Actual:
* Only Julian shows up.

And what shows up too is a logcat error :

E/GeckoConsole( 2982): [JavaScript Error: "TypeError: tels is null" {file: "app://sms.gaiamobile.org/js/thread_ui.js" line: 930}]

Seems both serious and easy to fix so requesting tef+ here.
note: I tested on v1-train and master. This is not related to the recent changes because the failing line exists since August 2012.
(Assignee)

Comment 4

6 years ago
Created attachment 737669 [details] [review]
Fix for 862061
Attachment #737669 - Flags: review?(felash)
(Assignee)

Comment 5

6 years ago
Wait, hold off on this... looks like the problem existed in more then one place
(Assignee)

Comment 6

6 years ago
@Julien this is good to go for review now
blocking-b2g: tef? → tef+
Whiteboard: [status: needs review julien]
Comment on attachment 737669 [details] [review]
Fix for 862061

r=me with the nit

ping me on the PR or with NEEDINFO when you're ready to merge (squash rebase and everything)
Attachment #737669 - Flags: review?(felash) → review+
Whiteboard: [status: needs review julien] → [status: needs follow-up, rebase and squash]
master: df823b637aabc4c7df28dc0ed3a09689e842da28
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [status: needs follow-up, rebase and squash] → [status: needs uplift]
This bug was partially uplifted.

Uplifted 862061 to:
v1-train: 24e818186cc9ab4e89783c6240c2003c36124387

Commit df823b637aabc4c7df28dc0ed3a09689e842da28 didn't uplift to branch v1.0.1
status-b2g18: --- → fixed
Rick, would you please try to fix the conflicts on 1.0.1 and push this on a branch in your repository ?
Flags: needinfo?(waldron.rick)
Rick, probably we'll need another completely different patch for landing on v1.0.1, because v1.0.1 does not have any of your changes to the contact search feature... Let's keep it as minimal as possible please :)
status-b2g18-v1.0.1: --- → affected
(Assignee)

Comment 13

6 years ago
@Julien, that seems like an incredibly bad idea. It will result in worse conflicts...
Flags: needinfo?(waldron.rick)
(Assignee)

Comment 14

6 years ago
The tests cannot be ported, as they rely on all of the test fixtures/mock as they appear in master.

Comment 16

6 years ago
Issue not occuring in Inari with the following build :

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/6bac24e14538
Gaia   c883af5ecd0998f78d9aaa4c2337c842e1dbb5a0
BuildID 20130416070200
Version 18.0
(In reply to Rick Waldron from comment #13)
> @Julien, that seems like an incredibly bad idea. It will result in worse
> conflicts...

at one point we want to have a stable branch and not uplift big patches ;)
Regarding the patch I've seen it's surgical for v1.0.1. Please add the comment requested and we could land  this. 
Please add your patch here as well and mark it as 'Patch for v1.0.1' in order to track it properly and give my R+. Thanks!
James, can we land this in v1.0.1. Thanks!
Flags: needinfo?(jlal)
I'll add the comment requested by Borja and land it.
Flags: needinfo?(jlal)
v1.0.1: 728fcbc5cd741083880d79fe74a84654696e618a

I also fixed the contact lookup that was probably broken since ages in v1.0.1 (an undeclared variable), I don't know why QA didn't catch this one.
status-b2g18-v1.0.1: affected → fixed

Comment 22

6 years ago
This issue is not reproducing. Verified and fixed on Inari
Inari Build ID:20130429070204
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/45aa5ba0ed53
Gaia: cf2d4136f0ebc66039637fdbeb72ed184dfbc0f2

This issue is not reproducing. Verified and fixed on Leo
Leo Build ID: 20130426070204
Kernel Date: Mar 15
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/6c2493de1441
Gaia: c9046a7acef33328977840176ff5574720d2c74c
Status: RESOLVED → VERIFIED
status-b2g18: fixed → verified
status-b2g18-v1.0.1: fixed → verified
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.