Alison Shiue Reporter
	Description • 11 years ago

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

	Kamil Leszczuk [:kamituel] Assignee
	Comment 1 • 11 years ago

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" ;)

	Francisco Jordano [:arcturus] [:francisco]
	Comment 2 • 11 years ago

Hi folks,

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

Thanks!

	Kamil Leszczuk [:kamituel] Assignee
	Comment 3 • 10 years ago

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

	Kamil Leszczuk [:kamituel] Assignee
	Comment 4 • 10 years ago

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

	Francisco Jordano [:arcturus] [:francisco]
	Comment 5 • 10 years ago

Comment on attachment 8437024 [details]
Pull request

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

	Michał Budzyński (:michalbe)
	Comment 6 • 10 years ago

Comment on attachment 8437024 [details]
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.

	Kamil Leszczuk [:kamituel] Assignee
	Comment 8 • 10 years ago

Thanks!

	Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]
	Comment 9 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 10 years ago

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

	Alison Shiue Reporter
	Comment 11 • 10 years ago

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

	Coler
	Comment 12 • 10 years ago

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