Open Bug 1325230 Opened 4 years ago Updated 1 year ago

`document.dir` is not set in about:config, preventing rtl from working

Categories

(GeckoView :: General, defect, P3)

Unspecified
Android
defect

Tracking

(firefox58 affected)

Tracking Status
firefox58 --- affected

People

(Reporter: itiel_yn8, Unassigned)

References

Details

Attachments

(3 files)

122.25 KB, image/png
Details
90.55 KB, image/png
Details
92.17 KB, image/png
Details
Attached image 1
When adding a new pref in about:config, the dialogs does not seem to support RTL.

Some screenshots attached.
Attached image 2
Attached image 3
See Also: → 1298913, 1332396, 1298904
Blocks: rtl-meta

Hi Itiel, could you please re-test this after this lands in Firefox Preview Nightly (around next Monday) ?

I think bug 1516577 should have solved this.

Depends on: 1516577
Flags: needinfo?(itiel_yn8)

This looks okay, but only because the whole layout is now LTR (including the New Preference dialog).

For example, this rule:
https://searchfox.org/mozilla-central/rev/b04e3a28a2ef4dbf957018dbbdc1840d62fdbc32/mobile/android/themes/geckoview/config.css#120-122
is not applied in RTL [1].

[1] Thank you for thinking about RTL, but this rule should never apply :)
In RTL the search icon should look the same as it is shown in your before/after screenshot in attachment 9119546 [details].
This makes me wonder why in the first place there is so much inconsistency with this icon, across the LTR UI..

Flags: needinfo?(itiel_yn8)

(In reply to Itiel from comment #4)

This looks okay, but only because the whole layout is now LTR (including the New Preference dialog).

That's really strange. I thought there was some JS that set the right dir attribute value on the root element...

I'll need to take a closer look later I guess. Thanks for testing!

Itiel, do you have any idea why the rtl mode is not being activated? (You can debug Fenix from about:debugging if you want).

When I manually added dir="rtl", the UI looked properly flipped, but I guess dir="rtl" is not getting applied at all in this case ?

https://searchfox.org/mozilla-central/rev/b04e3a28a2ef4dbf957018dbbdc1840d62fdbc32/mobile/android/chrome/geckoview/config.xhtml#26

That should be doing it, but for some reason it's not ?

Flags: needinfo?(itiel_yn8)

(In reply to Tim Nguyen :ntim (blocking ni? until Jan 22th) from comment #6)

Itiel, do you have any idea why the rtl mode is not being activated? (You can debug Fenix from about:debugging if you want).

Not sure.
Seeing that your patch didn't touch anything that might have regressed this, I'm wondering if this is caused by the recent work on the languages for Fenix [1][2], the timings seem to match.

[1] https://github.com/mozilla-mobile/fenix/issues/220
[2] https://github.com/mozilla-mobile/fenix/releases

(In reply to Itiel from comment #7)

Seeing that your patch didn't touch anything that might have regressed this, I'm wondering if this is caused by the recent work on the languages for Fenix [1][2], the timings seem to match.

Nope, that's not the case either.
I checked with a Fenix build from 2020-01-01 and the issue is still there, with the old design.

Tim, since you've essentially fixed this bug in bug 1516577 and the issue is really about dir="rtl" not getting added, is it better we close this one out and I'll file a bug in the fenix github repo?

Flags: needinfo?(itiel_yn8) → needinfo?(ntim.bugs)

(In reply to Itiel from comment #8)

Nope, that's not the case either.
I checked with a Fenix build from 2020-01-01 and the issue is still there, with the old design.

Thanks for checking!

Tim, since you've essentially fixed this bug in bug 1516577 and the issue is really about dir="rtl" not getting added, is it better we close this one out and I'll file a bug in the fenix github repo?

Not sure if dir='rtl' not being added is Fenix related or GeckoView related, maybe snorp knows (feel free to redirect if not) ?

Flags: needinfo?(ntim.bugs) → needinfo?(snorp)

It looks like document.dir isn't set at all in GeckoView apps. That's bad. I'll investigate some more.

Flags: needinfo?(snorp)
Component: Theme and Visual Design → General
Product: Firefox for Android → GeckoView
Summary: Adjust the New Preference dialog in about:config for RTL → `document.dir` is not set, preventing rtl pages from working correctly

Henri, do you know how document.dir ends up getting set on desktop? I'm not immediately finding it...

Flags: needinfo?(hsivonen)

Sorry, I don't how we set document.dir for in-content UI. Needinfoing jfkthame.

Flags: needinfo?(hsivonen) → needinfo?(jfkthame)

Also adding Zibi who might know.

Flags: needinfo?(gandalf)
Flags: needinfo?(gandalf)

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #14)

document.dir is set on Desktop via Fluent - https://searchfox.org/mozilla-central/source/dom/l10n/DOMLocalization.cpp#464-482

I had found that, but it appears that we guard running any of it with this[1], which seems to only apply to chrome/privileged pages. I must be missing something else?

[1] https://searchfox.org/mozilla-central/source/dom/base/nsContentUtils.cpp#1735

Flags: needinfo?(jfkthame) → needinfo?(gandalf)

We run it only on documents that use Fluent - which is triggered by the presence of <link rel="localization"/> in the head or <linkset></linkset> collection.

It should work in all documents from chrome, including unprivileged like about:neterrors and about:mozilla.

Flags: needinfo?(gandalf)
Priority: -- → P3
Summary: `document.dir` is not set, preventing rtl pages from working correctly → `document.dir` is not set in about:config, preventing rtl from working
You need to log in before you can comment on or make changes to this bug.