[Dialer] Autosuggestion is not working properly when a contact has two phone number with the same string

RESOLVED FIXED

Status

P2
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: isabelrios, Assigned: gduan)

Tracking

unspecified
x86_64
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

Details

(Whiteboard: leorun3, TaipeiWW)

Attachments

(4 attachments)

(Reporter)

Description

5 years ago
Leo device with testrun3 build

Test case id: #7316

PROCEDURE
Create a contact with two phone numbers: A: 12345678 and 1234901
Open Dialer app and start typing: 1234

EXPECTED
The autosuggestion panel result should show two entries for the same contact, one for each number matching the string typped

ACTUAL
Only the first number for contact A is shown as a result of typing 1234.
(Reporter)

Updated

5 years ago
blocking-b2g: --- → leo?
Gregor, do you know if this is front- or back-end?
blocking-b2g: leo? → leo+
Flags: needinfo?(anygregor)
So, I just tried, and it works for me on current master (see screenshot).

Can we know which gaia commit id was used in that build ?
Flags: needinfo?(anygregor)
I assume the contact API returns the whole contact to the app. This needs a frontend fix.
Flags: needinfo?(isabelrios)
Created attachment 762925 [details]
screen shot

(In reply to Isabel Rios [:isabel_rios] from comment #0)
> Leo device with testrun3 build
> 
> Test case id: #7316
> 
> PROCEDURE
> Create a contact with two phone numbers: A: 12345678 and 1234901
> Open Dialer app and start typing: 1234
> 
> EXPECTED
> The autosuggestion panel result should show two entries for the same
> contact, one for each number matching the string typped
> 
> ACTUAL
> Only the first number for contact A is shown as a result of typing 1234.

Also works for me.
To Rick and Julien,
What you reproduce is message app , but the actual issue is in dialer app which only show the first matching item of contact's tel information.
Assignee: nobody → gduan

Updated

5 years ago
Priority: -- → P2
Attachment #763503 - Flags: review?(etienne)
Comment on attachment 763503 [details]
Pointer to Github pull request https://github.com/mozilla-b2g/gaia/pull/10437

Hi Rex,

I think you're the best reviewer for this patch :)

(feel free to ping me/ask for feedback? at any point if you have some doubts)
Attachment #763503 - Flags: review?(rexboy)
Attachment #763503 - Flags: review?(etienne)
Attachment #763503 - Flags: review+

Updated

5 years ago
Whiteboard: leorun3 → leorun3, TaipeiWW
Comment on attachment 763503 [details]
Pointer to Github pull request https://github.com/mozilla-b2g/gaia/pull/10437

Hi George:
Most of them looks good for me, just a little concern on the _fillContacts(). I can r+ after the comment on Github are addressed.

Since there are changes on simplePhoneMatchers,
Etienne would you mind take a look on that?
Attachment #763503 - Flags: feedback?(etienne)
(Reporter)

Comment 10

5 years ago
(In reply to George Duan [:gduan] from comment #6)
> To Rick and Julien,
> What you reproduce is message app , but the actual issue is in dialer app
> which only show the first matching item of contact's tel information.

As George commented, the problem is seen from the dialer app.
It is still happening with today build, both v1-train (Gecko-2ff3151.Gaia-d8d189b)  and master (Gecko-2144715.Gaia-079b57f)
Thanks
Flags: needinfo?(isabelrios)
Comment on attachment 763503 [details]
Pointer to Github pull request https://github.com/mozilla-b2g/gaia/pull/10437

Thanks for pining me, comments are on github.
Attachment #763503 - Flags: feedback?(etienne)
Hi Etienne,

Code updated , would you mind to take a look again? Thanks.
Flags: needinfo?(etienne)
Comment on attachment 763503 [details]
Pointer to Github pull request https://github.com/mozilla-b2g/gaia/pull/10437

the simple phone matcher part looks good!
Attachment #763503 - Flags: feedback+
Flags: needinfo?(etienne)
Comment on attachment 763503 [details]
Pointer to Github pull request https://github.com/mozilla-b2g/gaia/pull/10437

Looks good for me now.
Thanks for your work, George!
Attachment #763503 - Flags: review?(rexboy) → review+
master:
https://github.com/mozilla-b2g/gaia/commit/06ea38cdd5d4fd098ed37bfada64c91135c406ef

Oh, I didn't notice the comment before merging..
I think it's better to squash your commits (or add bug numbers to each commit) next time.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 06ea38cdd5d4fd098ed37bfada64c91135c406ef
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(gduan)
Created attachment 767541 [details]
Pointer to Github pull request https://github.com/mozilla-b2g/gaia/pull/10621
Attachment #767541 - Flags: review?(jhford)
Flags: needinfo?(gduan) → needinfo?(jhford)
v1-train: b9d51a3
status-b2g18: --- → fixed
Flags: needinfo?(jhford)
Attachment #767541 - Flags: review?(jhford)
v1.1.0hd: b9d51a332bb68f705ad2d59614447f02f0441ed7
v1.1.0hd: 8e676640a18cc5c53c5d6dbffb582d671290fe80
status-b2g-v1.1hd: --- → fixed
You need to log in before you can comment on or make changes to this bug.