1013845 - [NFC][Contacts] The Chinese words receiver receive are garbled when share contact via NFC

Status: VERIFIED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Alison Shiue]

Attachment: S__1007618.jpg

Gaia      c462d9183d294a2d8ecc472f593ea8cfa15bc9de
Gecko     https://hg.mozilla.org/mozilla-central/rev/9d8d16695f6a
BuildID   20140520160203
Version   32.0a1

STR:
1. launch "settings" app for 2 phones
2. turn on "NFC" for 2 phones
3. device A create a new contact with some Chinese 
4. tap two phones to share the new contact created at step 3
5. check the contact device B received

Expected result:
contact information should correct 

Actual result:
Chinese is garbled

Comment 1

[Kamil Leszczuk [:kamituel]]

When I worked on bug 1003723 I noticed that NfcUtils.fromUTF8() and NfcUtils.toUTF8() aren't behaving correctly for non-ASCII characters.

Simple test:

> NfcUtils.toUTF8(NfcUtils.fromUTF8("łąka")) !== "łąka"  // true

There are couple of issues with current implementation:

1. When user enters input, it's not UTF-8 encoded. To achieve it, we should do something like: unescape(encodeURIComponent("łąka"))

2. This way, String.charCodeAt() will always return code < 256 which will be correctly stored in Uint8array. As of current implementation, char code for "ą" (for instance) returns 261, which is truncated when storing as 8 bit number.

3. NfcUtils.toUTF8() does not support characters consiting of more than 1 byte. It will treat them as separate characters.

4. contacts/../nfc.js does not use NfcUtils.fromUTF8(), but rather it's own implementation (also incorrect). This isn't really related to this bug, but should also be fixed in order for VCard sharing to be correct both ways.

ps. in case you're wondering, "łąka" means "meadow" ;)

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

Hi folks,

Changing the component to the NFC instead of contacts, since it's a general NFC problem.

Thanks!

Comment 3

[Kamil Leszczuk [:kamituel]]

I will fix points 1-3 in bug 1019169. Then fix this.

Comment 4

[Kamil Leszczuk [:kamituel]]

Attachment: Pull request

Fixes encoding issue by using NfcUtils.fromUTF8() instead of custom createBuffer() (which was buggy).

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8437024 [details] [review]
Pull request

Forwarding review to Michal since he knows NFC better than everyone else.

Comment 6

[Michał Budzyński (:michalbe)]

Comment on attachment 8437024 [details] [review]
Pull request

Tests are passing, bug is not occurring anymore (attachment), just one small nit commented on GH. Thanks Kamil, great job, working perfectly, r+ by me. Pls don't land before getting green Travis.

Comment 7

[Michał Budzyński (:michalbe)]

Attachment: Before & after screens

Comment 8

[Kamil Leszczuk [:kamituel]]

Thanks!

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://github.com/mozilla-b2g/gaia/commit/207cacb0dc5f320a02e7ef979233ebd4ab6bbbd5

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/48d28a33be050c0561becf029e0af37a8265d4bf

Comment 11

[Alison Shiue]

Verified on
Gaia      a3a5322692578e0a577fb7fa08e32144b2b05ba3
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/0293597de41f
BuildID   20140612160201
Version   32.0a2

Comment 12

[Coler]

This issue has been verified successfully on Flame2.1.
Reproducing rate: 0/5
See attachment: Verify_Flame_ContactNFC.mp4

Flame2.1 build version:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0

Comment 13

[Coler]

Attachment: Verify_Flame_ContactNFC.MP4