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

RESOLVED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::L10n
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: tedders1, Assigned: tedders1)

Tracking

unspecified
2.2 S12 (15may)
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
For more discussion of this issue, see the comments to Bug 1144682.
(Assignee)

Updated

3 years ago
Assignee: nobody → tclancy
Blocks: 1152074
(Assignee)

Updated

3 years ago
Depends on: 258974
(Assignee)

Comment 1

3 years ago
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.
Depends on: 922963
(Assignee)

Comment 2

3 years ago
*an imperfect solution
Whiteboard: [systemsfe]
(Assignee)

Updated

3 years ago
Depends on: 1161910
Created attachment 8601924 [details] [review]
[gaia] tedders1:bug-1154438-bidi-wrap-substitions > mozilla-b2g:master
(Assignee)

Comment 4

3 years ago
With the landing of Bug 1157726, gecko now supports FSI and PDI, so this can land now.
(Assignee)

Updated

3 years ago
No longer depends on: 258974, 922963
(Assignee)

Comment 5

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

Updated

3 years ago
Attachment #8601924 - Flags: review?(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.
Attachment #8601924 - Flags: review?(stas) → review+
(Assignee)

Comment 7

3 years ago
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.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Updated

3 years ago
Blocks: 1165443

Updated

3 years ago
status-b2g-master: --- → fixed

Updated

3 years ago
See Also: → bug 1165443

Comment 10

3 years ago
Hi Ted, 
Can you raise 2.2 approval as this seems to fix 2.2+ blocking bug 1166203?
Thanks!
blocking-b2g: --- → 2.2+
status-b2g-v2.2: --- → affected
Flags: needinfo?(tclancy)
Target Milestone: --- → 2.2 S12 (15may)
(Assignee)

Updated

3 years ago
Blocks: 1166203
(Assignee)

Comment 11

3 years ago
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.
Flags: needinfo?(tclancy)
Attachment #8601924 - Flags: approval-gaia-v2.2?

Updated

3 years ago
Attachment #8601924 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+

Comment 12

3 years ago
Hi Ryan. This is the last one. Thanks
Needs rebasing for v2.2 uplift.
Flags: needinfo?(tclancy)
(Assignee)

Comment 14

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

I'll apply for uplift on Bug 994357.
Depends on: 994357
Flags: needinfo?(tclancy)
You need to log in before you can comment on or make changes to this bug.