876754 - Use comma instead of | separator in contact auto suggestion

Status: RESOLVED FIXED

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

Description

[greg]

Per discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=873498

Comment 1

[greg]

How do you mock contacts to test the contact auto suggest?

Comment 2

[greg]

I mean mock them in the browser, not in unit tests.

Comment 3

[Julien Wajsberg [:julienw]]

Rick has put some useful mocks in his PR in https://github.com/mozilla-b2g/gaia/pull/9970.

But this bug should be very easy to fix: just change ' |' to ',' in [1]


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/thread_ui.js#L1386

Comment 4

[greg]

I agree that it should be simple but I don't feel comfortable making PRs that I can't test. Would really appreciate information on how to add contacts so I can test this.

Comment 5

[Julien Wajsberg [:julienw]]

I really don't know how to do that in the browser other than by using Rick's mocks that you can steal from his PR (because it won't land until next week). Otherwise, just test on the device where you can easily add contacts with the Contact app.

Comment 6

[Julien Wajsberg [:julienw]]

To make the contacts app work in the browser, you need to switch off the pref "dom.navigator-property.disable.mozContacts" in "about:config", and restart Firefox.

However, we still can't add contacts, this probably means there is a bug in Firefox for this, Alexandre Poirot is investigating this. So maybe soon in your browser without mocks ;)

Comment 7

[Julien Wajsberg [:julienw]]

"dom.global-constructor.disable.mozContact" need to be "false" too, restart nightly, and then everything works in the browser. yay !

Bug 877595 will put that in DEBUG=1 profiles soon !

Comment 8

[greg]

:julien This is awesome, I will try this. Thanks!

Comment 9

[greg]

This did not allow me to add contacts in nightly, sadly. However, Contacts app is now working on my phone. It was not working before, but I have pulled the latest master, so maybe this was related to something else that was fixed.

In any case, I can now test and fix this bug. PR will be soon.

Comment 10

[greg]

Attachment: Pull Request

Comment 11

[Corey Frang [:gnarf]]

Comment on attachment 756065 [details] [diff] [review]
Pull Request

Having problems with github for now, but this is a tiny simple fix - will merge when github returns to a working order.

Comment 12

[Corey Frang [:gnarf]]

master: 86d7da26f2a4468e46686006d5f09b09c6175db9

Comment 13

[Corey Frang [:gnarf]]

follow up commit on master: https://github.com/mozilla-b2g/gaia/commit/26b9f19d10c0009c2fc1c950fa3341dfa9e9235c

Comment 14

[Corey Frang [:gnarf]]

This should be on the train: https://bugzilla.mozilla.org/show_bug.cgi?id=873498#c14

Note to uplifters: there is a followup commit and the real commit:

in order:
master: https://github.com/mozilla-b2g/gaia/commit/86d7da26f2a4468e46686006d5f09b09c6175db9
master: https://github.com/mozilla-b2g/gaia/commit/26b9f19d10c0009c2fc1c950fa3341dfa9e9235c

Comment 15

[David Scravaglieri [:scravag]]

As per leo not a blocker.

Comment 16

[Julien Wajsberg [:julienw]]

I'd like to uplift this simple patch to prevent conflicts (eg bug 881248) and therefore making other uplifts safer. There is zero risk this patch can break anything, as it is only a string change.

Comment 17

[Dietrich Ayala (:dietrich)]

Partner triage agreed is safe (landed 24 days ago) and worth taking to reduce uplift friction.

Comment 18

[Corey Frang [:gnarf]]

uplifted masteR: 86d7da26f2a4468e46686006d5f09b09c6175db9
to v1-train: fc11358d986610ca128f85146da2fbe130b3ff65

uplifted master: 26b9f19d10c0009c2fc1c950fa3341dfa9e9235c
to v1-train: 04a668505226863d2749fe85e7057691e5093950

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: 04a668505226863d2749fe85e7057691e5093950
1.1hd: fc11358d986610ca128f85146da2fbe130b3ff65