With intl.l10n.pseudo = bidi, icons are not mirrored
Categories
(Core :: Internationalization, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox75 | --- | verified |
People
(Reporter: itiel_yn8, Assigned: zbraniecki)
References
Details
Attachments
(3 files)
intl.l10n.pseudo = bidi
+++ This bug was initially created as a clone of Bug #1597822 +++
STR:
- Create a
intl.l10n.pseudopref withbidias its contents.
AR:
The UI is flipped to look like RTL, but icons that are normally mirrored for RTL, aren't.
ER:
Icons should look as if intl.uidirection = 1.
This was supposedly fixed in bug 1597822 (and according to attachment 9110117 [details] everything looks fine with the patch) but this is still an issue.
At first I thought maybe this regressed since it was fixed on November 26th so I ran mozregression but I got to November 25th and I still see this being an issue.
|
Assignee | |
Comment 2•4 years ago
|
||
|
|
Assignee | |
Updated•4 years ago
|
|
|
Assignee | |
Comment 3•4 years ago
|
||
Itiel - thank you for reporting it!
I now have a patch that may address it. Can you test the build from https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa4c96e316731813a008ce636e9df2d34483e3f4 ?
Basically, when selecting document's directionality we used to look into ChromRegistry and check the "package". But we really didn't care about any other package than chrome, resource and about. And for those, we still really didn't expect their locale to be different than AppLocale.
But we used ChromeRegistry::IsLocaleRTL(package) test which doesn't check for intl.l10n.bidi since intl.l10n.bidi is not just directionality override, it's a locale override.
So with this patch, I'm switching us to check LocaleService::IsAppLocaleRTL for all packages we care about.
In result, with this patch I see the library icon, the fwd/back icons and the reload icon flipped. That seems to be an improvement.
One thing I noticed is that both with intl.l10n.bidi and with intl.uidirection we don't react to the switch updating the hamburger menu arrow icon, you need to open a new window after the switch to see the result.
That's likely worth filling as a new bug against the hambuger menu.
All in all, I'd expect with this patch the behavior of intl.uidirection and intl.l10n.pseudo to be aligned.
Please, verify that.
(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #3)
Itiel - thank you for reporting it!
I now have a patch that may address it. Can you test the build from https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa4c96e316731813a008ce636e9df2d34483e3f4 ?
All in all, I'd expect with this patch the behavior of
intl.uidirectionandintl.l10n.pseudoto be aligned.
Please, verify that.
Tested different bits of the Firefox UI and all seem well with this build, for both prefs :)
One thing I noticed is that both with
intl.l10n.bidiand withintl.uidirectionwe don't react to the switch updating the hamburger menu arrow icon, you need to open a new window after the switch to see the result.
That's likely worth filling as a new bug against the hambuger menu.
Hmm? I do know that there are several places where updating the prefs requires to open a new window / restart Firefox for them to function properly, but I'm not sure what you mean?
All arrow icons in the hamburger menu look okay to me immediately after updating the prefs...
I see some non-upside-down text when intl.l10n.bidi = bidi. I'm guessing all of these will become upside-down after the migration of Firefox to fluent will be completed?
|
|
Assignee | |
Comment 5•4 years ago
|
||
Hmm? I do know that there are several places where updating the prefs requires to open a new window / restart Firefox for them to function properly, but I'm not sure what you mean?
If you look at the screenshots you attached in this bug, in the hamburger menu by "Sign in to Firefox" the arrow ">" is pointing to the left in one picture, and to the right in the other.
I verified that this is not a difference between intl.uidirection and intl.l10n.pseudo but rather the fact that you need to open a new window after flipping either to trigger the arrow to point in the new direction.
Ok, thank you! I'll ask for review of this patch!
(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #5)
Hmm? I do know that there are several places where updating the prefs requires to open a new window / restart Firefox for them to function properly, but I'm not sure what you mean?
If you look at the screenshots you attached in this bug, in the hamburger menu by "Sign in to Firefox" the arrow ">" is pointing to the left in one picture, and to the right in the other.
I verified that this is not a difference betweenintl.uidirectionandintl.l10n.pseudobut rather the fact that you need to open a new window after flipping either to trigger the arrow to point in the new direction.
I'm confused, the screenshots I attached earlier were taken before testing with your patch and then the arrows are indeed pointing in the wrong direction, but now after testing with your patch (with either prefs) they are pointing to the correct direction without having to open a new window first. Are you not seeing the same?
Also, kindly answer the last question from comment 4 :-)
(I need to know if the recommendation in the following MDC doc for testers to test RTL using the intl.l10n.pseudo pref is misleading atm or not, and edit it accordingly:
https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/RTL_Guidelines#Testing )
|
|
Assignee | |
Comment 7•4 years ago
|
||
I'm confused, the screenshots I attached earlier were taken before testing with your patch and then the arrows are indeed pointing in the wrong direction, but now after testing with your patch (with either prefs) they are pointing to the correct direction without having to open a new window first. Are you not seeing the same?
No, With my patch you still will get the "wrong" direction of those arrows with both intl.uidirection and intl.l10n.pseudo if you switch the preference and check the menu in a window previously opened.
That is not related to this patch and is not fixed by it.
I see some non-upside-down text when intl.l10n.bidi = bidi. I'm guessing all of these will become upside-down after the migration of Firefox to fluent will be completed?
Yeah, only Fluent is pseudolocalized.
(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #7)
if you switch the preference and check the menu in a window previously opened.
Ah, that's what I was missing. Thanks.
|
||
Updated•4 years ago
|
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/87347a74981d Use AppLocale to set document direction for chrome, about and resource protcols. r=jfkthame
|
||
Comment 10•4 years ago
|
||
Backed out changeset 87347a74981d (Bug 1614767) for failures in test_righttoleft.xhtml
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=289722902&resultStatus=testfailed%2Cbusted%2Cexception&revision=87347a74981d3a75b91b08162640712125c64aa0
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=289722902&repo=autoland&lineNumber=8773
Backout: https://hg.mozilla.org/integration/autoland/rev/e40fad30bd8256b2b5815d13ffe4d7edcedd040a
|
|
Assignee | |
Comment 11•4 years ago
|
||
Ugh... this tests actually relies on the idea that we can override locale in chrome registry - https://searchfox.org/mozilla-central/source/toolkit/content/tests/chrome/rtltest/righttoleft.manifest#3
:jfkthame - would you be ok with me removing it? I don't think it makes sense anymore to try to salvage it as we are actively moving away from ChromeRegistry overall?
|
||
Comment 13•4 years ago
|
||
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/127a116c7f53 Use AppLocale to set document direction for chrome, about and resource protcols. r=jfkthame
|
||
Comment 14•4 years ago
|
||
| bugherder | ||
Description
•