1080828 - [Messages][RTL] International phone numbers and groupmms threads are not correctly displayed

Status: VERIFIED FIXED

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

Description

[Julien Wajsberg [:julienw]]

International phone numbers are shown as 1234567890+ instead of [phone].
Headers for groupmms threads are "(FirstName LastName (+1" (IIRC) instead of "(1+) FirstName LastName". I'm not sure how "FirstName LastName" should be ordered in RTL when it's written in a LTR language. NI Ahmed about this.

There are a lot of ways to fix this:

* force "direction: ltr; unicode-bidi: embed" (or "bidi-override", maybe on the parent, I'm not sure)
* force isolation using either <bdi> or "unicode-bidi: -moz-isolate" or "isolate-override".

W3C thinks isolate should be used instead of embed (see [1]), so it's probably the way to go.

[1] http://www.w3.org/International/wiki/Html-bidi-isolation#Other_alternatives_that_were_rejected

Comment 1

[Ahmed Nefzaoui [:Nefzaoui]]

Hey Julien :)

For he international phone numbers & group mms headers:
If the correct English format is "(1+) FirstName LastName"
Arabic should look like "LastName FirstName (1+)"

Comment 2

[Julien Wajsberg [:julienw]]

Hey Ahmed,

the correct english format would be "FirstName LastName (+1)" (means: FirstName LastName and 1 other recipient).

My main interrogation is, in RTL, if "FirstName LastName" is using LTR (for example, my name), because I think it should be expected that people using the phone in RTL will still have some contacts in LTR.

If "FirstName LastName" is in RTL (an arabic name for example), then it's easy, I don't have any question :) But if "FirstName LastName" is in LTR (a french name), then I don't know.

BTW we can have the same question when using the phone in LTR mode, but displaying an arabic name. I _think_ we should have "FirstName LastName" displayed as a block.

So, maybe I can try to make things concrete.

Say you have a contact "محمد نسبة" (sorry if it's not correct, I used some google translate and wikipedia words ;) ).

محمد is the first name
نسبة is the last name

(and I know that in the arabic world, there are generally more than 2 names, but let's keep things simple for now :) )

And you have a contact "Jacques Chirac"

"Jacques" is the first name
"Chirac" is the last name

How should:
* "محمد نسبة" be displayed in LTR mode? I think it should be displayed just like that, and that we should not try to reverse the words.
* "Jacques Chirac" be displayed in RTL mode? I think the same: it should be displayed as one, and we should not reverse the words.

Ahmed, Ehsan, what do you think ? :)

Comment 3

[Ahmed Nefzaoui [:Nefzaoui]]

> the correct english format would be "FirstName LastName (+1)" (means: FirstName LastName and 1 other recipient).
Then in Arabic we should expect seeing "(+1) LastName FirstName" (assuming we don't put any extra work for manually re-ordering FirstName & LastName, this is automatically done even if they're not concatenated or each word is from a different source :) ).

> * "محمد نسبة" be displayed in LTR mode? I think it should be displayed just like that, and that we should not try to reverse the words.
Exactly. :) My point is, we shouldn't try to manually try to reverse names as this will be automatically done. It's the same ordering, just from a different perspective (English from the left, Arabic from the right).

> * "Jacques Chirac" be displayed in RTL mode? I think the same: it should be displayed as one, and we should not reverse the words.
Yes, As it is, non-RTL languages content needs to stay the same.

Thanks!! :)

Comment 4

[Julien Wajsberg [:julienw]]

Ok, then we're on the same page, great :)

Comment 5

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL (wip)

Current WIP patch, unit tests are broken yet

Comment 6

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8515800 [details] [review]
GitHub pull request URL (wip)

Hey Julien,

Could you please take a look at the patch and give your precious early feedback? :)

I have some doubts/questions that I've summarized at GitHub. Don't pay attention to failing unit tests -  I haven't fixed it yet.

Thanks!

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 8515800 [details] [review]
GitHub pull request URL (wip)

looks good !

Comment 8

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8515800 [details] [review]
GitHub pull request URL (wip)

(In reply to Julien Wajsberg [:julienw] from comment #7)
> Comment on attachment 8515800 [details] [review]
> GitHub pull request URL (wip)
> 
> looks good !

Thanks for feedback! Added separate commit that fixes unit tests + some other updates.

With the latest PR I don't see issue with header font-size you've mentioned at Github [1]. Please, let me know if you're still experiencing this.

[1] https://github.com/mozilla-b2g/gaia/pull/25749#issuecomment-61522208

Comment 9

[Julien Wajsberg [:julienw]]

Comment on attachment 8515800 [details] [review]
GitHub pull request URL (wip)

r=me with the small nit

I'd also like to have RTL names (arabic or hebrew) in the contact mock (and some matching conversations of course, including group MMS), can you add this before landing?

Comment 10

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #9)
> Comment on attachment 8515800 [details] [review]
> GitHub pull request URL (wip)
> 
> r=me with the small nit
> 
> I'd also like to have RTL names (arabic or hebrew) in the contact mock (and
> some matching conversations of course, including group MMS), can you add
> this before landing?

Thanks for review! I've fixed nit and added desktop mocks you suggested, but as it turned out bug 1094320 can a real blocker for the approach I use to construct bidirectional heading in Thread panel. <gaia-header> can simply replace entire <h1>'s innerHTML with plain text content at some circumstances (see bug 1094320 comment 0 for more details).

Comment 11

[Oleg Zasypkin [:azasypkin]]

As agreed offline, if we'll have to do anything to workaround bug 1094320, we'll do in a separate bug.

Master: https://github.com/mozilla-b2g/gaia/commit/0f93196939e02a559ed23ca4ade6301d3bc9a7de

Comment 12

[Sue]

Attachment: verify_pass.png

This issue has been verified as pass on latest build of Flame 2.2&3.0 and Nexus 5 2.2&3.0 by STRs:
Prerequisite:
1. Insert a SIM card and launch Message.
2. Send a MMS to a group containing an international phone number like "[phone]".
3. Observe the heading at message dialog view.
4. Tap the heading and observe the view.
Results:
Step3: The heading is shown as "(+1) LastName FirstName".
Step4: The international phone number is shown correctly like "[phone]".
See attachment:verify_pass.png
Rate:0/3

Device: Flame 2.2 (pass)
Build ID               20150525002504
Gaia Revision          144673a413586f98b5e2c27b781c1a539611f754
Gaia Date              2015-05-25 02:01:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c4db2af40b1b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150525.041143
Firmware Date          Mon May 25 04:11:54 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (pass)
Build ID               20150525160205
Gaia Revision          5bcc08a732163087999251b523e3643db397412c
Gaia Date              2015-05-24 14:44:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/b6623a27fa64
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150525.192755
Firmware Date          Mon May 25 19:28:07 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (pass)
Build ID               20150525002504
Gaia Revision          144673a413586f98b5e2c27b781c1a539611f754
Gaia Date              2015-05-25 02:01:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c4db2af40b1b
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150525.041303
Firmware Date          Mon May 25 04:13:19 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (pass)
Build ID               20150525160205
Gaia Revision          5bcc08a732163087999251b523e3643db397412c
Gaia Date              2015-05-24 14:44:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/b6623a27fa64
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150525.192207
Firmware Date          Mon May 25 19:22:24 EDT 2015
Bootloader             HHZ12f