Closed Bug 1543093 Opened 6 years ago Closed 6 years ago

[RTL] Browser Toolbox entire layout is reversed from the normal DevTools

Categories

(DevTools :: General, defect, P3)

defect

Tracking

(firefox68 verified)

VERIFIED FIXED
Firefox 68
Tracking Status
firefox68 --- verified

People

(Reporter: itiel_yn8, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

(Keywords: rtl)

Attachments

(5 files)

See title.
The Browser Toolbox (Ctrl+Alt+Shift+I) layout should be the same as the normal DevTools (Ctrl+Shift+C) for RTL locales.

Browser Toolbox items that should be RTL'd:

  1. Inspector
  2. Debugger
  3. Memory
  4. Network
  5. DOM
  6. Accessibility
  7. DevTools settings page
  8. Re-ordering of the first and last tab in the DevTools is broken (the right one is draggable only to the right, and the left one only to the left)
  9. Tooltips (of the main buttons of the UI, at least. Some tooltips of actual debugging stuff should remain LTR). For example, Accessibility's tab tooltip

See screenshots for general comparison.

Attached image Inspector - Browser Toolbox (deleted) —
Attached image Inspector - DevTools (deleted) —
Attached image Memory - Browser Toolbox (deleted) —
Attached image Memory - DevTools (deleted) —

It took me some time to understand the problem so let me rephrase it here: in the browser toolbox, the panels' contents aren't reversed as they correctly are in the in-browser devtools interface.

The priority flag is not set for this bug.
:pbro, could you have a look please?

Flags: needinfo?(pbrosset)

Hey Julian, do you think this has anything to do with the host window types we use? I know we've done some work on loading our panels in content iframes, which sounded like it might have an impact on how RTL was handled.

To be very clear, here's what's happening using an RTL version of Firefox:

  • in the normal toolbox, the inspector is displayed in RTL mode (the 3 panels are displayed in this order: Layout|Rules|Markup, and various text fields and buttons are RTL too)
  • in the browser toolbox, the inspector displays the 3 panels in this order instead Markup|Rules|Layout and nothing is RTL'd really.
Flags: needinfo?(pbrosset) → needinfo?(jdescottes)
Priority: -- → P2

We have not landed any of the changes to the frame type.

Browser toolbox launches a separate Firefox instance (and uses a separate profile).
My guess is that the following method doesn't detect RTL in this special instance of Firefox:

  loader.lazyGetter(this, "direction", () => {
    // Get the direction from browser.xul document
    const top = this.topWindow;
    const topDocEl = top.document.documentElement;
    const isRtl = top.getComputedStyle(topDocEl).direction === "rtl";
    return isRtl ? "rtl" : "ltr";
  });

searchfox

If you want to test this locally, you can try to force pref("intl.uidirection", 1); in modules/libpref/init/all.js to force your build to use RTL.

I added logs in loader.lazyGetter(this, "direction",. As suspected it returns a different value for the BrowserToolbox than for the regular toolbox.

We use a dedicated profile for the Browser Toolbox. Therefore I checked the value of the preference "intl.uidirection". But it is correctly set even in the BrowserToolbox profile. For some reason the topmost window of the Browser Toolbox is not "rtl". But I don't know why. The process for the BrowserToolbox is started at devtools/client/framework/ToolboxProcess.js

The method here uses the "top" window of the toolbox. If instead we use the actual toolbox window "rtl" will work on the Browser Toolbox:

  loader.lazyGetter(this, "direction", () => {
    const isRtl = this.win.getComputedStyle(this.doc.documentElement).direction === "rtl";
    return isRtl ? "rtl" : "ltr";
  });

We started using the topmost window to compute "direction" as early as Bug 1305007. I don't think there was a strong reason to pick this one rather than the toolbox window. Let's change it to use the toolbox window.

Note on the priority, I think it should rather be P3. Browser Toolbox is normally not used for anything except Firefox development. Extension development was the last external use case for the Browser Toolbox, but we switched it to regular toolboxes in FF68. It's not a recent regression (as I said, probably here since Bug 1305007).

Flags: needinfo?(jdescottes)

Thank you so much Julian for the investigation and describing a solution here.
I'll change this to P3 as you suggested. I think that makes sense.
I'm going to mark you as mentor on this bug in order to get it on some medium time contributor's radars, but feel free to reset that flag if you don't want or have the bandwidth to mentor.

Mentor: jdescottes
Priority: P2 → P3

Using the toolbox window to read the "direction" seems to fix the issue.
Added a mochitest, had to create a dedicated browser ini file in order to setup RTL properly.

Assignee: nobody → jdescottes
Mentor: jdescottes
Status: NEW → ASSIGNED

(sorry, started writing a patch afterall :) )

Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b2bb8867a1b5 Fix RTL for DevTools in Browser Toolbox;r=nchevobbe
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

Pretty much perfect, thank you!

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: