1154438 - String substitutions should be wrapped with Unicode bidi isolation characters when appropriate.

Status: RESOLVED FIXED

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

Description

[Ted Clancy [:tedders1]]

For more discussion of this issue, see the comments to Bug 1144682.

Comment 1

[Ted Clancy [:tedders1]]

There's been some discussion about whether we should use <bdi> and </bdi> tags for this, or Unicode FSI and PDI control characters.

Bug 1152127 is a good example of where <bdi> and </bdi> tags might not work. Unfortunately, gecko doesn't currently support FSI and PDI. (Support will be added by 922963, but it's unclear on when that will be finished.)

In the meantime, and imperfect solution using LRE and RLE might be better than nothing. Unfortunately, LRE and RLE don't provide perfect isolation.

Comment 2

[Ted Clancy [:tedders1]]

*an imperfect solution

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] tedders1:bug-1154438-bidi-wrap-substitions > mozilla-b2g:master

Comment 4

[Ted Clancy [:tedders1]]

With the landing of Bug 1157726, gecko now supports FSI and PDI, so this can land now.

Comment 5

[Ted Clancy [:tedders1]]

(This patch currently produces test failures in the FTU. Those will be fixed by Bug 1161910.)

Comment 6

[Staś Małolepszy :stas]

Comment on attachment 8601924 [details] [review]
[gaia] tedders1:bug-1154438-bidi-wrap-substitions > mozilla-b2g:master

Thanks, Ted, r=me with a nit left in github about the fat arrow.

IIUC, this will ensure proper directionality of LTR text inside of RTL translations.  It won't do anything for RTL text inside of LTR translations, but that's much less common anyways and probably already taken care of with <bdi> where absolutely needed, like in Contacts.

Comment 7

[Ted Clancy [:tedders1]]

Hi Staś. 

The patch does take care of RTL substitutions inside an LTR translation. On line 911, I check for 'value.match(nonLatin1)'. That looks in the substitution string for a non-Latin-1, possibly RTL character.

Comment 8

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/951b02671d40bb7f7ddca85ab77765aae10812f3

Comment 9

[Staś Małolepszy :stas]

L20n.js: https://github.com/l20n/l20n.js/commit/41a1b726ad9eff872671d94606adf7c7300d60f6

Comment 10

[Josh Cheng [:josh]]

Hi Ted, 
Can you raise 2.2 approval as this seems to fix 2.2+ blocking bug 1166203?
Thanks!

Comment 11

[Ted Clancy [:tedders1]]

Comment on attachment 8601924 [details] [review]
[gaia] tedders1:bug-1154438-bidi-wrap-substitions > mozilla-b2g:master

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
RTL support for B2G (Bug 906270)

This patch is required for Bug 1166203, which is confirmed as blocking 2.2. (And probably a bunch of similar issues which haven't been spotted yet.)

User impact if declined: 
Punctuation marks will appear in the wrong place when LTR phrases appear within RTL text, or vice versa.

Testing completed: 
Green try run - https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=1365157b38eb59bb42fd32f63c9eb5254e4d4a67

Risk to taking this patch (and alternatives if risky): 
None foreseen.

String or UUID changes made by this patch:
None.

Comment 12

[Josh Cheng [:josh]]

Hi Ryan. This is the last one. Thanks

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v2.2 uplift.

Comment 14

[Ted Clancy [:tedders1]]

Hi Ryan. This patch needs to land on top of Bug 994357.

I'll apply for uplift on Bug 994357.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/428bd8dfebff803c549dfae5e4f9dc695131652b