883770 - [Dialer][SMS][Contacts] Call log does no longer show contact names after version upgrade

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/f1b84d841ced
Gaia   0b0cd9ee3eb9fb09f8505214615492bcc5eff7da
BuildID 20130616230206
Version 18.0

Since I have updated my phone from the build below, the call does no longer show contact names. Only the phone numbers called are visible. This is a regression between builds 20130613230207 and 20130616230206.

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/537f3eac4c48
Gaia   0c836be7f24693b74ad7fd21c8153824f1c7f548
BuildID 20130613230207
Version 18.0

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Might be this is caused by a Gecko change? In the call log I only see normalized phone numbers without brackets and spaces while in my contacts every one has those special characters.

Comment 2

[Julien Wajsberg [:julienw]]

Bug 871930 removed some magic we had in Dialer, not sure this is related (because it landed before your 20130613 build). Maybe Etienne will have more information.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This even doesn't work for local numbers without the country prefix and any brackets nor spaces. Just a number like 0351123456 will fail to show the contact name in the call log.

Comment 4

[Gregor Wagner [:gwagner]]

Can you try to delete the contact and add it again? This might be an issue of a missing DB migration code.

Comment 5

[Marcia Knous [:marcia]]

Henrik: Can you add a screenshot of what you see? Parul has been testing this today to see if she can reproduce.

Comment 6

[Etienne Segonzac (:etienne)]

(In reply to Gregor Wagner [:gwagner] from comment #4)
> Can you try to delete the contact and add it again? This might be an issue
> of a missing DB migration code.

Yep. The call log is working okay.
Looks like a migration issue :/

Comment 7

[Julien Wajsberg [:julienw]]

same thing for SMS by the way.

* the contact would not match on a number (no difference in internationalization, space, etc, it was a plain internationalized number both in sms and in contact)
* I updated the contact, changed a number and put it back
=> it was matching again.

Comment 8

[Julien Wajsberg [:julienw]]

I'd like to add that my contacts were freshly imported contacts from GMail, so I don't think it was a migration problem for me, but I may miss something.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Julien Wajsberg [:julienw] from comment #7)
> both in sms and in contact)
> * I updated the contact, changed a number and put it back
> => it was matching again.

Yeah, I can confirm that! I have only updated the name of the contact and checked the call log and sms list. Now the name is displayed correctly again. So what kind of migration issue could this be? Have we updated the schema of the contacts database lately, and with the update of the contact we correctly write out the information?

If that is the case it seems to be very important to get fixed before any of our users will get updated from 1.0 to 1.1.

I don't think the request for a screenshot is still valid? Given that others do also see that.

Comment 10

[Julien Wajsberg [:julienw]]

We need leo+ on this, lots of contact matching is broken right now because of this.

Comment 11

[Gregor Wagner [:gwagner]]

We know what's going on here. It's the missing migration code for the match field.
Reuben can you take this?

Comment 12

[Reuben Morais [:reuben]]

What missing migration?

https://mxr.mozilla.org/mozilla-central/source/dom/contacts/fallback/ContactDB.jsm#329

Comment 14

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 15

[Gregor Wagner [:gwagner]]

Ah how much do I love this upgrade scenario.
The bad thing is that this doesn't solve the problem for people that already see this bug. This will only solve it for users that haven't upgraded yet.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Would another update of the schema help here?

Comment 17

[Gregor Wagner [:gwagner]]

(In reply to Henrik Skupin (:whimboo) [away 06/28 - 07/07] from comment #16)
> Would another update of the schema help here?

Yeah I was thinking more about it. We should make the buggy upgrade code a noop and bump the revision number again with a correct upgrade code.
Michael is taking care of it :)

Comment 18

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Based on comment 11 and having a patch, removing regressionwindow-wanted.

Comment 19

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

Attachment: Patch with upgrade path fix

Comment 20

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/projects/birch/rev/56172e239ed5

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/56172e239ed5

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/c01b9ca8d218

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/c01b9ca8d218

Comment 24

[Gregor Wagner [:gwagner]]

Attachment: Followup

My mistake :(

Comment 25

[Reuben Morais [:reuben]]

Comment on attachment 769995 [details] [diff] [review]
Followup

Review of attachment 769995 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch.

Comment 26

[Gregor Wagner [:gwagner]]

Followup:
https://hg.mozilla.org/releases/mozilla-b2g18/rev/b9547f9afe33
https://hg.mozilla.org/projects/birch/rev/f921263b8cc7

Comment 27

[Ryan VanderMeulen [:RyanVM]]

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

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/b9547f9afe33

Comment 29

[Julien Wajsberg [:julienw]]

I wonder if there should be another follow up, because the 'match' search does not match anything now.

(however, 'contains' search still works ok)

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Whatever has been done here, this is not fixed on my Unagi for the nightly channel:

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18/rev/39607fd11f6b
Gaia   d336288e6cda1a8f974ceea41b9d7860c2367d1f
BuildID 20130706230209
Version 18.0

Comment 31

[Julien Wajsberg [:julienw]]

I verified this works for new contact but still not for the old ones :(

Michael, Gregor, can I somehow investigate what's inside the indexeddb ?

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Julien, I assume that we should reopen the  bug completely, or? I don't think that it will be fixed on trunk either.

Comment 33

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

Julien, you can see what's in the contacts DB by using bent's IndexedDB FF extension: https://addons.mozilla.org/en-US/firefox/addon/indexeddb-browser/

To use the extension for this purpose, you will need to first adb pull the contacts sqlite file off the phone and place it in your FF profile folder for indexeddb (also make sure to delete any .DS_Store files if you're using MacOS finder to move stuff around). Restart your browser, then open the IndexedDB browser (Tools->Web Developer->IDB Browser), and drill into the DB named "contacts." Inside "contacts" there will be another "contacts" entry, click on this and click the Load Data button. Here you should see all your contacts. If you examine the the value of one, you should see a "parsedTel" entry in the JSON string. If it's missing, the upgrade probably failed. Let me know if you encounter any hiccups, cause I had to do some wrangling to get it to work for me.

Reopening this bug for further investigation.

Comment 34

[Julien Wajsberg [:julienw]]

It seems that the problem comes with using |this.substringMatching| in most places except the migration path.

Therefore, for a number [phone], we try to match 23456789 whereas only [phone], 123456789 and 0123456789 are in the index. However, adding/updating or importing generates the index correctly.

Comment 35

[Jason Smith [:jsmith]]

Guys - Can you file a followup bug for this?

Comment 36

[Julien Wajsberg [:julienw]]

Right,

Could use the just-filed Bug 890909 (that I was going to dupe ;) ).

Comment 37

[Gregor Wagner [:gwagner]]

(In reply to Julien Wajsberg [:julienw] from comment #34)
> It seems that the problem comes with using |this.substringMatching| in most
> places except the migration path.
> 
> Therefore, for a number [phone], we try to match 23456789 whereas only
> [phone], 123456789 and 0123456789 are in the index. However,
> adding/updating or importing generates the index correctly.

Hm I don't see how this can reduce the search space. The substringMatching only adds the last x digits to the search space but we should still find the number with the national formats. 
What format is 23456789 in your example? Isn't 0123456789 or 123456789 the local format?

Comment 38

[Julien Wajsberg [:julienw]]

It reduces the search space because the value that is looked for is only the sliced value... which is not in the index.

see https://mxr.mozilla.org/mozilla-central/source/dom/contacts/fallback/ContactDB.jsm#893

When investigating with Mike, we found that being in France I should not be using the sliced value, but because "init" is called too soon, we always end up with the default brazil configuration. 

But that's another issue, and we should file another bug for this. I also don't understand why we don't always slice for all countries... because we don't want to match 2 similar numbers from 2 different countries?

Comment 39

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Julien, why did you move the status to affected again? I thought we have bug 890909 for the followup work now?

Comment 40

[Julien Wajsberg [:julienw]]

completely right, I mixed up my bugs, sorry.