1053709 - Make SMS messages content UI RTL-friendly

Status: RESOLVED FIXED

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

Comment 1

[Julien Wajsberg [:julienw]]

I think we'll need to use either <bdi> or `dir="auto"` attribute, or the CSS property "unicode-bidi: -moz-isolate/isolate-override/plaintext", I'm not sure which one yet. I think we'll need to play with real arabic or hebrew messages to understand what we need to use.

Comment 2

[Julien Wajsberg [:julienw]]

We need to check also RTL message in a LTR document, and LTR message in RTL document, and even RTL messages containing LTR content and LTR messages containing RTL content. Lots of cases to check (would be easier with data in the desktop mock).

We need to handle the bubble position only if the spec is clear. If it's not let's do this in a separate bug.

Comment 3

[Steve Chung [:steveck]]

Attachment: Link to github

Hi Julien, based on bidi meeting last week we will need to mirror the bubble layout, so I adjust the bubble and attachment layout for RTL. For the text content, I use unicode-bidi: -moz-plaintext to force the text always display the correct direction on matter user choose which language. Using dir="auto" might lead similar result, but there is still a major difference while bubble contains both ltr and rtl language. dir="auto" will only apply 1 direction for entire content, but -moz-plaintext could apply different direction depends on the text, which I think would be more appropriate.

Hi Ahmed, could you please help verify if this patch works as you expected? Thanks!

Comment 4

[Julien Wajsberg [:julienw]]

Comment on attachment 8524519 [details] [review]
Link to github

I tried various other values instead of "-moz-plaintext" and I also tried 'dir="auto"' but in the end it's what you've done that looks better.

Let's see what Ahmed will say :) Ahmed, can you try these cases:
* messages that have both RTL and LTR content
* a message that starts with arabic, and another that starts with latin text (but still contains both type of content)
* using a phone configured in arabic, and a phone configured in english

Thanks!

Comment 5

[Steve Chung [:steveck]]

Comment on attachment 8524519 [details] [review]
Link to github

Some nit addressed, hope we can land this patch in this sprint and create follow up if necessary.

Comment 6

[Steve Chung [:steveck]]

Attachment: rtl.jpg

Hi Jenny, it's about the UI display for RTL text thread list under LTR environment. It sounds reasonable to set the direction depends on the text content, but it might look weird if we only move the recipient and message text to right and leave others at left. Ahmed, we will need you opinion from Arabic user POV to know if it's necessary to change for this case, thanks.

Comment 7

[Steve Chung [:steveck]]

Attachment: rtl-content.jpeg

It's the result from current patch if the message contains both LTR and RTL text. We set different direction in bubble depend on text ,and we thought it's the best approach for now. Please let us know if you prefer unified direction for whole messages based on the system language or unified direction per message based on the first word of message, thanks.

Comment 8

[Ahmed Nefzaoui [:Nefzaoui]]

Apologies for my late reply. My only concern for now here is the use of -moz-dir() for the outgoing and incoming bubbles. If we are *ONLY* mirroring their positions shouldn't we just use html[dir="rtl"] ?

(In reply to Steve Chung [:steveck] from comment #6)
> Created attachment 8526640 [details]
> rtl.jpg
> 
> Hi Jenny, it's about the UI display for RTL text thread list under LTR
> environment. It sounds reasonable to set the direction depends on the text
> content, but it might look weird if we only move the recipient and message
> text to right and leave others at left. Ahmed, we will need you opinion from
> Arabic user POV to know if it's necessary to change for this case, thanks.

I think we should just keep it that way. Don't know about what would UX say but if we're gonna think about handling both RTL and LTR cases for every single direction, we should probably mirror all the elements of a single message depending on the text content (time, date, number/recipient etc), but that then will create confusion as the user start to have RTL and non-RTL messages and will create more complex use cases. (hope my explanation is clear enough? :S)

(In reply to Steve Chung [:steveck] from comment #7)
> Created attachment 8526642 [details]
> rtl-content.jpeg
> 
> It's the result from current patch if the message contains both LTR and RTL
> text. We set different direction in bubble depend on text ,and we thought
> it's the best approach for now. Please let us know if you prefer unified
> direction for whole messages based on the system language or unified
> direction per message based on the first word of message, thanks.

From my POV: PERFECT.
Yes, it may look weird (not so much though) but from a readability perspective it's what we should actually be doing...

Comment 9

[Julien Wajsberg [:julienw]]

Ahmed, if you can try the patch in real conditions on your phone, it would be awesome :)
Of course it could be done later as well, and you could file follow-up bugs if you think things could get improved.

NI Ahmed for this, but take your time!

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 8524519 [details] [review]
Link to github

I'm afraid this doesn't work properly with -moz-plaintext once we have RTL and LTR text in the same sentence. We need to find other solutions :/

Comment 11

[Julien Wajsberg [:julienw]]

(In reply to Steve Chung [:steveck] from comment #6)
> Created attachment 8526640 [details]
> rtl.jpg
> 
> Hi Jenny, it's about the UI display for RTL text thread list under LTR
> environment. It sounds reasonable to set the direction depends on the text
> content, but it might look weird if we only move the recipient and message
> text to right and leave others at left. Ahmed, we will need you opinion from
> Arabic user POV to know if it's necessary to change for this case, thanks.

(In reply to Steve Chung [:steveck] from comment #6)
> Created attachment 8526640 [details]
> rtl.jpg
> 
> Hi Jenny, it's about the UI display for RTL text thread list under LTR
> environment. It sounds reasonable to set the direction depends on the text
> content, but it might look weird if we only move the recipient and message
> text to right and leave others at left. Ahmed, we will need you opinion from
> Arabic user POV to know if it's necessary to change for this case, thanks.



What Oleg did for recipients is having "text-align: right" if parent is ":-moz-dir(rtl)" (and "text-align: left" otherwise). I don't know if that could help you here though.

Comment 12

[Steve Chung [:steveck]]

Attachment: moz-plaintext-ltr.jpeg

If the a sentence contains bot LTR and RTL language and starting word's direction is different from the system's direction, I admit that it looks weird if we force the whole sentence in same direction. But if the different direction word is show up right in the middle, it becomes even more weird for my POV. I'll attach another example later. Ni? Ahmed for the correct display direction for LTR/RTL mix up.

Comment 13

[Steve Chung [:steveck]]

Attachment: moz-plaintext.jpeg

It's another example about the RTL/LTR mix up. The display with -moz-plaintext will force the sentence's direction defined by the first word of sentence. Still NI? Ahmed for the correct display direction.

Comment 14

[Jenny Lee]

Hey Steve, I think the reading experience should be as intuitive as possible for RTL readers, even if we need to slightly compromise on the visual aspect. let's follow what Ahmed said here. thanks!

Comment 15

[Stephany Wilkes]

Ahmed, we need feedback to determine if this can land in 2.2 or not.

Comment 16

[Ahmed Nefzaoui [:Nefzaoui]]

Sorry for the way-too-much late reply :(

(In reply to Steve Chung [:steveck] from comment #13)
> Created attachment 8527476 [details]
> moz-plaintext.jpeg
> 
> It's another example about the RTL/LTR mix up. The display with
> -moz-plaintext will force the sentence's direction defined by the first word
> of sentence. Still NI? Ahmed for the correct display direction.

Concerning this:
with -moz-plaintext in RTL: perfectly correct. :)
without -moz-plaintext in RTL: quite very messed up, should not be like this.
with -moz-plaintext in LTR: Also correct.
without -moz-plaintext in LTR: Not correct.

(In reply to Steve Chung [:steveck] from comment #12)
> Created attachment 8527472 [details]
> moz-plaintext-ltr.jpeg
> 
> If the a sentence contains bot LTR and RTL language and starting word's
> direction is different from the system's direction, I admit that it looks
> weird if we force the whole sentence in same direction. But if the different
> direction word is show up right in the middle, it becomes even more weird
> for my POV. I'll attach another example later. Ni? Ahmed for the correct
> display direction for LTR/RTL mix up.

The example with -moz-plaintext makes much more sense since the messages starts with Arabic, logically the whole content starts right and then aligns to whatever we have else within the message. So yes, with -moz-plaintext is LTR is the correct one :)

(In reply to Steve Chung [:steveck] from comment #7)
> Created attachment 8526642 [details]
> rtl-content.jpeg
> 
> It's the result from current patch if the message contains both LTR and RTL
> text. We set different direction in bubble depend on text ,and we thought
> it's the best approach for now. Please let us know if you prefer unified
> direction for whole messages based on the system language or unified
> direction per message based on the first word of message, thanks.

From my perspective, we should do different text direction depending on text. So attachment id=8526642 makes perfect sense :)

Comment 17

[Ahmed Nefzaoui [:Nefzaoui]]

Comment on attachment 8524519 [details] [review]
Link to github

Looking good, tried every possible case and it looks like everything is working fine.
However, I'm finding another bug a little annoying; It would be better if we have the text field aligned to the direction of the first character written in it :)

Comment 18

[Steve Chung [:steveck]]

(In reply to Ahmed Nefzaoui [:Nefzaoui] from comment #17)
> Comment on attachment 8524519 [details] [review]
> Link to github
> 
> Looking good, tried every possible case and it looks like everything is
> working fine.
> However, I'm finding another bug a little annoying; It would be better if we
> have the text field aligned to the direction of the first character written
> in it :)

Thanks for the feedback! Could you please clarify the issue you faced? Maybe a image for discription would be better. We might fix right in this patch if issue is not difficult, otherwise we can fire another bug for it.

Comment 19

[Steve Chung [:steveck]]

Comment on attachment 8524519 [details] [review]
Link to github

Hi Oleg, the patch would works fine based on the Ahmed's feedback(except thte one issue he mentioned above). Since Julien might busy on other stuff and you also have lots of experience on RTL, could you please take a final look for it? Thanks!

Comment 20

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8524519 [details] [review]
Link to github

Looks good to me, thanks!

Comment 21

[Steve Chung [:steveck]]

Thanks!
In master: 1672a719e5cac08f51beb92ada4670066c0cf9c7