Closed Bug 1494582 Opened 1 year ago Closed 1 year ago

[RTL]Font editor: Toggle switch for "Italic" on RTL builds is spilling out of its designated area

Categories

(DevTools :: Inspector: Fonts, defect, P2)

defect

Tracking

(firefox-esr60 unaffected, firefox62 unaffected, firefox63 verified, firefox64 verified)

VERIFIED FIXED
Firefox 64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- verified
firefox64 --- verified

People

(Reporter: csasca, Assigned: rcaliman)

References

Details

(Keywords: regression)

Attachments

(2 files)

Attached image Toggle Switch.gif
[Affected versions]:
- Firefox Beta 63.0b9
- Firefox Nightly 64.0a1 (2018-09-26)

[Affected platforms]:
- Windows 10 (x64)
- macOS 10.13
- Ubuntu 16.04 (x64)

[Steps to reproduce]:
1.Open Firefox
2.Navigate to about:config
3.Set the following value to true: devtools.chrome.enabled
4.Set intl.uidirection value to 1
5.Restart Firefox
6.Navigate to about:newtab
7.Open "Developer Toolbar" using CTRL+SHIFT+I combination
8.Navigate to the "Fonts" sub-section in the left window of the inspector.
9.Click on "Italic" toggle switch

[Expected result]:
- The toggle switch is moving to left according to the RTL ui.

[Actual result]:
- The switch is keeping the left to right behavior, which makes it spill out of the toggle area.

[Additional notes]:
The actual behavior for the toggle switch can be seen on the attachment.
I'm back with the regression range:

Last good revision: 83d0673bbca49e9713f5326708cf9908f2d71764
First bad revision: f5389dab7be85e3a2d7e1a68a2d39869e22e8795
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=83d0673bbca49e9713f5326708cf9908f2d71764&tochange=f5389dab7be85e3a2d7e1a68a2d39869e22e8795

Bug 1476580 seems to be the culprit.
Keywords: regression
Thanks for filing, this is likely due to this line of code https://searchfox.org/mozilla-central/rev/6ddb5fb144993fb5de044e2e8d900d7643b98a4d/devtools/client/themes/common.css#839
which unconditionally moves the toggle towards the right, whether the UI is in rtl or ltr.
We probably need a second CSS rule prefixed with html[dir="rtl"] to override it.
Priority: -- → P2
By P2, do you mean a fix for 63, 64, or later?
Flags: needinfo?(pbrosset)
Assignee: nobody → rcaliman
@pbro

Thank you for the tip, Patrick! I submitted a patch to fix this. Since the toggle is a shared component, the Flexbox Inspector will benefit from this fix as well.

@jcristau

> By P2, do you mean a fix for 63, 64, or later?

Since it's a trivial fix, we can perhaps uplift this for the Firefox Beta 63 cycle.
Flags: needinfo?(pbrosset)
Sorry, I meant to reply to David there, not Julien.
Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e28a0bc4a7c5
Adjust toggle input direction for right-to-left layout; r=pbro
(In reply to David Durst [:ddurst] (Regression Engineering Owner for 63) from comment #3)
> By P2, do you mean a fix for 63, 64, or later?
Let's try to get it uplifted to 63, since that's where we're shipping the new Font Editor. We want to make sure it is as robust as we can for all our users (including rtl users) right when it hits releases, not one version later.
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/e28a0bc4a7c5
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Please request Beta approval on this when you get a chance.
Flags: needinfo?(rcaliman)
Comment on attachment 9014075 [details]
Bug 1494582 - Adjust toggle input direction for right-to-left layout; r=pbro

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1494582

User impact if declined: Users in RTL languanges will see a broken UI for the italic toggle in the new Font Editor feature in DevTools. This will be the first time Stable users get to see the tool. It's desirable to make a good first impression.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Not risky because it's only a CSS tweak to ensure the toggle button doesn't look broken on RTL layout. Other layouts are left unchanged.

String changes made/needed:
Flags: needinfo?(rcaliman)
Attachment #9014075 - Flags: approval-mozilla-beta?
Comment on attachment 9014075 [details]
Bug 1494582 - Adjust toggle input direction for right-to-left layout; r=pbro

Low risk, uplift approved for 63 beta 13, thanks.
Attachment #9014075 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
The issue is no longer reproducible on Firefox Beta 63.0b13 and latest Firefox 64.0a1 (2018-10-09). Tests were performed on Windows 10 (x64), macOS 10.13 and Ubuntu 16.04 (x64).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.