813834 - Toolbar of Facebook Message is character corruption when using Japanese facebook messenger

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Makoto Kato [:m_kato]]

toolbar data is stored into prefs.js (social.cached.notificationIcons).  

getCharPref and setCharPref is "string", not "wstring".  So we need Text(Encoder|Decoder) or nsIScriptableUnicodeConverter.

Current code use JSON parser directly, so social.cached.notificationIcons will corrupt if using non-ASCII.

Comment 2

[Makoto Kato [:m_kato]]

Attachment: v1

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Seems like it would be easier to switch to an nsISupportsString complex pref, and just invalidate the cache once (e.g. by switching the pref name)?

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(thanks for filing this - good catch!)

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 684345 [details] [diff] [review]
v1

Thanks for filing and writing the patch. Can you redo this patch using the setComplexPref/getComplexPref approach? https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIPrefBranch#setComplexValue%28%29

We'll also want to change the pref name as Gavin suggested.

Comment 6

[Makoto Kato [:m_kato]]

Attachment: fix v2

get/setComplexValue instead of

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 686019 [details] [diff] [review]
fix v2

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

Thanks! Looks good to me.

Comment 8

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/089e5412a83b

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/089e5412a83b

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/089e5412a83b

Comment 11

[Makoto Kato [:m_kato]]

Comment on attachment 686019 [details] [diff] [review]
fix v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none.
User impact if declined: Tooltip of Facebook toolbar is still corrupted if using Facebook non-US version.  Facebook (and another partner for Social API) cannot localize tooltip/label of toolbar.
Testing completed (on m-c, etc.): landed.  Also this has test.
Risk to taking this patch (and alternatives if risky): too low.  This fix stores correct cache into prefs.js.
String or UUID changes made by this patch: no

Comment 12

[:Gavin Sharp [email: [e-mail]]]

I agree that we should take this on Aurora/Beta.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6bdd53a2301c
https://hg.mozilla.org/releases/mozilla-beta/rev/cdf68610254e

Comment 14

[u279076]

Marking verified fixed since this landed with tests. Please add verifyme keyword if there's something you need QA to test.