Closed Bug 1125602 Opened 10 years ago Closed 10 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 verified, b2g-master verified)

VERIFIED DUPLICATE of bug 1124582
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: azasypkin, Unassigned)

References

Details

(Keywords: regression, rtl)

Attachments

(1 file)

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)")
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!
Flags: needinfo?(wilsonpage)
Cc Ahmed so that he knows about this.
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.
Flags: needinfo?(swilkes)
(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
Flags: needinfo?(wilsonpage)
Flags: needinfo?(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?
Flags: needinfo?(azasypkin)
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
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).
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.
Flags: needinfo?(swilkes)
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?
Flags: needinfo?(swilkes)
(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.
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.
Flags: needinfo?(swilkes) → needinfo?(anygregor)
(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.
Flags: needinfo?(anygregor)
(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.
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.
Ok, looks like I misunderstood Kaze's comment 13, so it looks like we all agree :)
I respectfully agree with you too. :)
This is fixed by the patch in bug 1124582.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Test case has been added in moztrap: https://moztrap.mozilla.org/manage/case/15933/
Flags: in-moztrap+
Attached image Verify1_master.png (deleted) —
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: