1125602 - [Messages][RTL] Header text of group MMS thread should respect directionality of the current locale

Status: VERIFIED DUPLICATE of bug 1124582

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

Description

[Oleg Zasypkin [:azasypkin]]

Looks like patch for bug 1102138 regressed RTL-compatibility of group MMS header content. See example below and bug 1094822 on how it should look like:

LTR:
* John (+3) (currently is "John (+3)")
* أحمد‎ (+3) (currently is "أحمد‎ (+3)")

RTL:
* (+3) John (currently is "John (+3)")
* ‎(+3) أحمد (currently is "أحمد‎ (+3)")

Comment 1

[Oleg Zasypkin [:azasypkin]]

Hey Wilson,

Could you please look into it? I'm wondering if patch for bug 1102138 should only prevent reversing of gaia-header's direct children (eg. actions buttons) and not entire nested content.

Thanks!

Comment 2

[Julien Wajsberg [:julienw]]

Cc Ahmed so that he knows about this.

Comment 3

[Ahmed Nefzaoui [:Nefzaoui]]

Yet another bug.
I really think we shouldn't be doing this in the first place (I mean not mirroring headers).
It's true, by reversing we are making compromises like, being unconsistent with how edge gestures work (as I heard from UX).
But by not mirroring them we're making bigger compromises by being oddly different from what a native Arabic (or any RTL-based language) user would expect, and the simple user may or may not understand why we do not mirror headers but will surely be confused by the fact that back button (for example) which is an indicator of going back to the past is actually positioned at where a future-action indicator is located (since it's Right-To-Left then Right is the beginning, Left is End, thus before Right text goes past actions, after Left content goes future actionable items).
So instead of patching again and again the flaws of not mirroring heads and doing work-arounds for them, I genuinely recommend shipping 2.2 with mirrored headers.

Comment 4

[Wilson Page [:wilsonpage]]

(In reply to Oleg Zasypkin [:azasypkin] from comment #1)
> Hey Wilson,
> 
> Could you please look into it? I'm wondering if patch for bug 1102138 should
> only prevent reversing of gaia-header's direct children (eg. actions
> buttons) and not entire nested content.
> 
> Thanks!

So in RTL you want 'John (+3)' to become '(+3) John'? I guess the issue is we already set the entire component to `direction: ltr` meaning all descendents get that styling too. I guess we'd need a rule like:

:host-context([dir=rtl]) ::content h1 {
  direction: rtl;
}

If that is correct I can make the patch to gaia-header

Comment 5

[Oleg Zasypkin [:azasypkin]]

(In reply to Wilson Page [:wilsonpage] from comment #4)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #1)
> > Hey Wilson,
> > 
> > Could you please look into it? I'm wondering if patch for bug 1102138 should
> > only prevent reversing of gaia-header's direct children (eg. actions
> > buttons) and not entire nested content.
> > 
> > Thanks!
> 
> So in RTL you want 'John (+3)' to become '(+3) John'? 

Exactly, as it was before!

> I guess the issue is we already set the entire component to `direction: ltr` meaning all
> descendents get that styling too. I guess we'd need a rule like:
> 
> :host-context([dir=rtl]) ::content h1 {
>   direction: rtl;
> }
> 
> If that is correct I can make the patch to gaia-header

Sorry, I'm not sure that I understand ":host-context([dir=rtl])" correctly, does it mean that we should have something like this "<gaia-header dir="rtl"><h1>....</h1></gaia-header>" to match? We have dir attribute on "html" element only, maybe ":-moz-dir(rtl)" will work?

Comment 6

[Wilson Page [:wilsonpage]]

Last time I checked :-moz-dir() didn't reflect the document's dir when used in a shadow-dom stylesheet. Host context selector is used to reference an ancestor's selector outside of the shadow-dom. It's not implemented yet, but I have a partial polyfil as part of gaia-component [1].

[1] http://github.com/gaia-components/gaia-component

Comment 7

[Julien Wajsberg [:julienw]]

For reference, :host-context implementation is bug 1082060 and is a 2.2 blocker.
There is a bug at W3C's bugzilla about directionality inheritance in shadow DOMs: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27359 (I don't know if we have one in our bugzilla, I think we do).

Comment 8

[Stephany Wilkes]

We're not shipping 2.2 with mirrored headers. I'm sorry - this is a decision we made for several reasons, not just transitions but for all of the work it creates for individual app teams. If each app were using the same header, it would be less of an issue, but they're not (AFAIK). And the individual app teams couldn't commit to getting the work done, possibly creating a case in which some headers would be mirrored and others would not be.

If these points are moot, let me know, but as far as I know they're still issues.

Comment 9

[Julien Wajsberg [:julienw]]

Hey stephany AFAIK all apps are using the same header, but I may be wrong. Do you know any app where it's not the case?

Comment 10

[Fabien Cazenave [:kaze]]

(In reply to Julien Wajsberg [:julienw] from comment #9)
> Hey stephany AFAIK all apps are using the same header, but I may be wrong.

All apps I’ve checked so far use the same <gaia-header> component, and we even have a patch ready to mirror it. The counter part is that to ensure the consistency with the “back” arrow, we would have to ensure all in-apps panel transitions are RTL-ified as well — which is rather straight-forward but depends on every app.

Comment 11

[Stephany Wilkes]

Well, and it's not just panel transitions: it's those, plus Sheets and the Task Manager as well. 

Flagging Gregor just in case he thinks we can and should change all of these transitions for 2.2 but, given how buggy 2.2 already is and with FL in less than a month, I'm still leaning toward no despite what the header might give us.

Comment 12

[Gregor Wagner [:gwagner]]

(In reply to Stephany Wilkes from comment #11)
> Well, and it's not just panel transitions: it's those, plus Sheets and the
> Task Manager as well. 
> 
> Flagging Gregor just in case he thinks we can and should change all of these
> transitions for 2.2 but, given how buggy 2.2 already is and with FL in less
> than a month, I'm still leaning toward no despite what the header might give
> us.

Given our current timeline its more important to narrow down the scope and reduce risk. If anyone can convince me that mirroring reduces risk and scope lets talk but otherwise lets focus on the issues that block us from shipping.

Comment 13

[Fabien Cazenave [:kaze]]

(In reply to Gregor Wagner [:gwagner] from comment #12)
> Given our current timeline its more important to narrow down the scope and
> reduce risk. If anyone can convince me that mirroring reduces risk and scope
> lets talk but otherwise lets focus on the issues that block us from shipping.

To be perfectly honest, I’m afraid that mirroring the headers and panel transitions will *not* reduce the risk for the 2.2 release.

Comment 14

[Julien Wajsberg [:julienw]]

I respectfully disagree with you here, Kaze. I think we'll have a long tail of behavior inconsistency that would be blocker bugs if we decide to implement this. That's exactly the reason why Stephany and Gregor choose to not do it.

Please attach your patch in another bug if it's not done yet. This would make it easier for someone to fix the issue on master when there is time to do that.

And let's fix this simple bug with a future-proof 3-line patch.

Comment 15

[Julien Wajsberg [:julienw]]

Ok, looks like I misunderstood Kaze's comment 13, so it looks like we all agree :)

Comment 16

[Fabien Cazenave [:kaze]]

I respectfully agree with you too. :)

Comment 17

[Julien Wajsberg [:julienw]]

This is fixed by the patch in bug 1124582.

Comment 18

[Alissa（leave from Mozilla）]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15933/

Comment 19

[Lancy(Leave from Mozilla)]

Attachment: Verify1_master.png

This issue is verified as pass on latest Flame v2.2/master and N5 v2.2/master.
STR:
Precondition: One or two sim cards are inserted in device.
1.Launch Messages and enter new message view.
2.Send the group MMS and observe the header.
**The number of the member in parenthesis is shown at left side of the contact name/ phone number.

See attachment:Verify1_master.png.
Reproducing rate:0/5

Device: Flame v2.2 build (Pass)
Build ID               20150722162505
Gaia Revision          e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gaia Date              2015-07-15 21:05:11
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a73051740290
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150722.195353
Firmware Date          Wed Jul 22 19:54:05 EDT 2015
Bootloader             L1TC000118D0

Device: Flame master build (Pass)
Build ID               20150722160203
Gaia Revision          f04fdbfa1943dddeab8ecd1299a76ab56e590d00
Gaia Date              2015-07-22 18:44:09
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/8650fe82f1cd
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150722.194012
Firmware Date          Wed Jul 22 19:40:25 EDT 2015
Bootloader             L1TC000118D0

Device: N5 v2.2 build (Pass)
Build ID               20150722002505
Gaia Revision          e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gaia Date              2015-07-15 21:05:11
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a73051740290
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150722.040945
Firmware Date          Wed Jul 22 04:10:05 EDT 2015
Bootloader             HHZ12f

Device: N5 master build (Pass)
Build ID               20150722160203
Gaia Revision          f04fdbfa1943dddeab8ecd1299a76ab56e590d00
Gaia Date              2015-07-22 18:44:09
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/8650fe82f1cd
Gecko Version          42.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150722.192830
Firmware Date          Wed Jul 22 19:28:49 EDT 2015
Bootloader             HHZ12f