1606263 - Tooltips are displayed as LTR on RTL builds

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Itiel]

STR:

1. Use RTL version of Nightly (he, ar, fa). Also make sure intl.uidirection is set to 1
2. Do any of the following:
   a. when on bugzilla, hover the lock icon on the urlbar
   b. on a non-bookmarked webpage, hover the star button on the urlbar
   c. hover the Firefox Account button
   d. hover the More Tools... chevron icon on the main toolbar

AR:
All tooltips should appear as RTL

ER:
They're displayed as if they're LTR

Regressed by bug 1598841.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Ah, nice catch.

This is kind of expected. The tooltip was not supposed to inherit from the root element, and it just happened to. So it needs to be styled explicitly.

So a CSS rule with moz-locale-dir ought to do it, or we could just set the dir attribute on them as appropriate...

I'm away from my computer, but tooltip:-moz-native-anonymous:-moz-locale-dir(rtl) { direction: rtl } in something like xul.css or xul-minimal.css ought to do (has to be a UA sheet).

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1606263 - Make default tooltip rtl when appropriate. r=Itiel

After bug 1598841, they don't inherit direction from the root, so we need to
explicitly style the default toltip.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

That should do (tested that it changes the appearance of the tooltips and such with intl.uidirection=1), but please let me know if it needs more work :-)

Comment 4

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]: rlt regression

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc6505aeeb65
Make default tooltip rtl when appropriate. r=Itiel

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9118026 [details]
Bug 1606263 - Make default tooltip rtl when appropriate. r=Itiel

Beta/Release Uplift Approval Request

• User impact if declined: Tooltips appear wrongly positioned in RTL languages.

I know we're really close to the 72 RC, but this should probably still get uplifted.

• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple CSS change to correct accidental regression.
• String changes made/needed: none

Comment 7

[Julien Cristau [:jcristau]]

fix for a rtl regression caused by an earlier 72 uplift, let's take this in 72 rc1.

Comment 8

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/e3f0d87af6e1

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1606263 - Test-only follow-up: Fix browser_parsable_css to account for UA-only selector.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03ed5ed6cba7
Test-only follow-up: Fix browser_parsable_css to account for UA-only selector.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Does https://phabricator.services.mozilla.com/D58386 also need uplift? Can you request uplift on that patch to beta/release if that makes sense? Thanks!

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9118066 [details]
Bug 1606263 - Test-only follow-up: Fix browser_parsable_css to account for UA-only selector.

Beta/Release Uplift Approval Request

• User impact if declined: none, test-only change
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• 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): test-only change to keep treeherder happy
• String changes made/needed: none

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/bc6505aeeb65
https://hg.mozilla.org/mozilla-central/rev/03ed5ed6cba7

Comment 14

[Alin Ilea, Desktop QA]

Reproduced the issue on Nightly 73.0a1 (Build id: 20191229212642).
Verified-fixed on latest Nightly 73.0a1 (Build id: 20191230214931) and Beta 72.0 (Build id: 20191230161130) using Windows 10, Mac OS 10.14 and Ubuntu 18.04.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 9118066 [details]
Bug 1606263 - Test-only follow-up: Fix browser_parsable_css to account for UA-only selector.

test-only, no need for approval

Comment 16

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/776b6180af5f