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)
Tracking
(b2g-v2.2 verified, b2g-master verified)
VERIFIED
DUPLICATE
of bug 1124582
People
(Reporter: azasypkin, Unassigned)
References
Details
(Keywords: regression, rtl)
Attachments
(1 file)
(deleted),
image/png
|
Details |
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)")
Reporter | ||
Comment 1•10 years ago
|
||
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)
Comment 2•10 years ago
|
||
Cc Ahmed so that he knows about this.
Comment 3•10 years ago
|
||
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)
Comment 4•10 years ago
|
||
(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)
Updated•10 years ago
|
Flags: needinfo?(azasypkin)
Reporter | ||
Comment 5•10 years ago
|
||
(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)
Comment 6•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
||
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)
Comment 9•10 years ago
|
||
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)
Comment 10•10 years ago
|
||
(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•10 years ago
|
||
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)
Comment 12•10 years ago
|
||
(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)
Comment 13•10 years ago
|
||
(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•10 years ago
|
||
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•10 years ago
|
||
Ok, looks like I misunderstood Kaze's comment 13, so it looks like we all agree :)
Comment 16•10 years ago
|
||
I respectfully agree with you too. :)
Comment 17•10 years ago
|
||
This is fixed by the patch in bug 1124582.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Comment 18•10 years ago
|
||
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15933/
Flags: in-moztrap+
Comment 19•9 years ago
|
||
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
Updated•9 years ago
|
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
status-b2g-v2.2:
--- → verified
status-b2g-master:
--- → verified
You need to log in
before you can comment on or make changes to this bug.
Description
•