Closed
Bug 790192
Opened 9 years ago
Closed 9 years ago
B2G SMS: support automatically replacing Latin characters with ASCII ones
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: vicamo, Assigned: vicamo)
Details
(Keywords: feature, Whiteboard: [LOE:S])
Attachments
(2 files, 6 obsolete files)
16.26 KB,
patch
|
Details | Diff | Splinter Review | |
11.82 KB,
patch
|
Details | Diff | Splinter Review |
See Gaia issue https://github.com/mozilla-b2g/gaia/issues/3298. For some countries, they want to use GSM 7-bit default alphabet encoding for characters that can be replaced with similar-in-shape ASCII ones. The sent message stored should be still consisted of Latin characters while the received message is partially or fully in ASCII.
Assignee | ||
Comment 1•9 years ago
|
||
Characters to be replaced: Á(\u00C1) => A(\u0041), á(\u00E1) => a(\u0061), Í(\u00CD) => I(\u0049), í(\u00ED) => i(\u0069), Ó(\u00D3) => O(\u004F), ó(\u00F3) => o(\u006F), Ú(\u00DA) => U(\u0055), ú(\u00FA) => u(\u0075), ç(\u00E7) => Ç(\u00C7)
Comment 2•9 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #1) > Characters to be replaced: [...] > ç(\u00E7) => Ç(\u00C7) You probably meant: ç(\u00E7) => c(???) Right?
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Mounir Lamouri (:mounir) from comment #2) > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #1) > > Characters to be replaced: > [...] > > ç(\u00E7) => Ç(\u00C7) > > You probably meant: > ç(\u00E7) => c(???) > > Right? Actually, Ç(capital cedilla) is in GSM 7-bit default but ç(lower case cedilla) is not. Beatriz and I will confirm this again.
Comment 4•9 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #3) > (In reply to Mounir Lamouri (:mounir) from comment #2) > > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #1) > > > Characters to be replaced: > > [...] > > > ç(\u00E7) => Ç(\u00C7) > > > > You probably meant: > > ç(\u00E7) => c(???) > > > > Right? > > Actually, Ç(capital cedilla) is in GSM 7-bit default but ç(lower case > cedilla) is not. Beatriz and I will confirm this again. Checked today with the help of vicamo, the suitable change is: > > > ç(\u00E7) => Ç(\u00C7) ç(lower case cedilla) is not in GSM 7-bit default Code Scheme.
Comment 5•9 years ago
|
||
That sounds really weird. In French, I would prefer to read "garcon" instead of "garÇon"...
Comment 6•9 years ago
|
||
Portuguese and catalan people are used to see Ç, they prefer it because it is not a gramatical mistake only a minor mistake.
Comment 7•9 years ago
|
||
In Portuguese and Spanish languages, the default code scheme for SMS should be 7 bit. The reason is the number of characters included in a SMS is 160. The latin characters that are not covered with the 7 bit default scheme are replaced as described in comment #1. Please mark this issue as blocking because it is important for platform customers.
Assignee | ||
Updated•9 years ago
|
blocking-basecamp: --- → ?
Assignee | ||
Comment 8•9 years ago
|
||
Do we must have a Settings UI for this? Or it's fine to be a product specific, read-only flag? Or a mutable preference setting like "dom.sms.strict7bitEncoding"?
Comment 9•9 years ago
|
||
Is this going to make feature freeze? Otherwise I think we'll have to - this.
blocking-basecamp: ? → +
Assignee | ||
Updated•9 years ago
|
Whiteboard: [LOE:S]
Assignee | ||
Comment 11•9 years ago
|
||
Assignee | ||
Comment 12•9 years ago
|
||
Assignee | ||
Comment 13•9 years ago
|
||
Attachment #666177 -
Attachment is obsolete: true
Attachment #666416 -
Flags: review?(marshall)
Assignee | ||
Comment 14•9 years ago
|
||
Attachment #666178 -
Attachment is obsolete: true
Attachment #666417 -
Flags: review?(marshall)
Assignee | ||
Comment 15•9 years ago
|
||
let the map be string to string.
Attachment #666416 -
Attachment is obsolete: true
Attachment #666416 -
Flags: review?(marshall)
Attachment #666484 -
Flags: review?(marshall)
Assignee | ||
Comment 16•9 years ago
|
||
add test cases to verify the map
Attachment #666417 -
Attachment is obsolete: true
Attachment #666417 -
Flags: review?(marshall)
Attachment #666485 -
Flags: review?(marshall)
Comment 17•9 years ago
|
||
Comment on attachment 666484 [details] [diff] [review] Part1: v2 Review of attachment 666484 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, r=me with comments addressed. ::: b2g/app/b2g.js @@ +385,5 @@ > pref("dom.ipc.browser_frames.oop_by_default", false); > > // Temporary permission hack for WebSMS > pref("dom.sms.enabled", true); > +//pref("dom.sms.strict7BitEncoding", true); // Disabled by default. nit: should we uncomment this and set it to false? ::: dom/system/gonk/RadioInterfaceLayer.js @@ +1952,5 @@ > return options; > }, > > getNumberOfMessagesForText: function getNumberOfMessagesForText(text) { > + let strict7BitEncoding = Services.prefs.getBoolPref("dom.sms.strict7BitEncoding"); IIRC this will throw an exception if the pref doesn't exist. We should handle that gracefully here (and below in sendSMS) ::: dom/system/gonk/ril_consts.js @@ +1621,5 @@ > + map["\u00FA"] = "\u0075"; // ú(\u00FA) => u(\u0075) > + map["\u00E7"] = "\u00C7"; // ç(\u00E7) => Ç(\u00C7) > + > + return map; > +})(); Any reason this can't be a literal object instead of creating and discarding a function scope?
Attachment #666484 -
Flags: review?(marshall) → review+
Updated•9 years ago
|
Attachment #666485 -
Flags: review?(marshall) → review+
Assignee | ||
Comment 18•9 years ago
|
||
address review comment #17: 1. Uncomment pref() call and set it to false. This does make it more clear. 2. Wrap call to getBoolPref with try..catch 3. Use literal object instead. Thank you :)
Attachment #666484 -
Attachment is obsolete: true
Assignee | ||
Comment 19•9 years ago
|
||
re-formated in hg
Attachment #666485 -
Attachment is obsolete: true
Assignee | ||
Comment 20•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f1ea4ac1aa4 https://hg.mozilla.org/integration/mozilla-inbound/rev/97d7f66e2b30
Comment 21•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6f1ea4ac1aa4 https://hg.mozilla.org/mozilla-central/rev/97d7f66e2b30
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in
before you can comment on or make changes to this bug.
Description
•