1313751 - Fix unicode bidi for URL of tooltip in RTL locales

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[magicp]

Attachment: fix-unicode-bidi-for-url-of-tooltip.png

Steps to Reproduce:
1. Start any versions Firefox in RTL locales (e.g. Arabic)
2. Open "https://www.mozilla.org/en-US/contribute/"
3. Bookmark current page
4. Mouse hover on urlbar and this bookmark > Check URL of tooltip

Actual Results:
"/https://www.mozilla.org/en-US/contribute"
 ^
Expected Results:
"https://www.mozilla.org/en-US/contribute/"
                                         ^

Comment 1

[:Gijs (he/him)]

Do we do this wrong for other tooltips or tooltip-like items, such as the link hovering tooltip, or tooltips on the history or "restore closed tabs" menus / panels ?

Comment 2

[magicp]

Do you think need to modify each tooltips?

Comment 3

[magicp]

Attachment: Firefox-vs-Chrome-vs-Edge-in-RTL-locales.png

Comment 4

[:Gijs (he/him)]

(In reply to magicp from comment #2)
> Do you think need to modify each tooltips?

Well, probably - if we do this wrong in other tooltips, which was my question. It would be helpful if you could answer that. :-)

Comment 5

[magicp]

We should fix them if Firefox supports with RTL locales. I think tooltips should be displayed original text simply.

Comment 6

[:Gijs (he/him)]

(In reply to magicp from comment #5)
> We should fix them if Firefox supports with RTL locales. I think tooltips
> should be displayed original text simply.

But you've not answered the question... Of course we should fix this, what I'm trying to find out is whether we need to basically fix 1 tooltip that is broken compared to 5 others that are working correctly, or whether ALL of them are broken, and what we really want is to make XUL automatically set direction:auto on tooltips or something like that. For this, it would be *really helpful* if you could clarify if it's *only* the bookmarks tooltips that are broken, or also the other ones.

Comment 7

[magicp]

I don't know how to find all. But if all tooltips are controlled by global/popup.css, you can change direction. can't you?

Comment 8

[:Gijs (he/him)]

(In reply to magicp from comment #7)
> I don't know how to find all.

Sure, but the history and synced tabs ones would be a good start.

> But if all tooltips are controlled by
> global/popup.css, you can change direction. can't you?

"Maybe."

It might not actually be appropriate to change the direction of *all* of these items.

Neil, do you have ideas about what would be the easiest solution here?

Comment 9

[magicp]

(In reply to :Gijs Kruitbosch from comment #8)

> It might not actually be appropriate to change the direction of *all* of
> these items.

Do you know any wrong cases?

Comment 10

[:Gijs (he/him)]

(In reply to magicp from comment #9)
> (In reply to :Gijs Kruitbosch from comment #8)
> 
> > It might not actually be appropriate to change the direction of *all* of
> > these items.
> 
> Do you know any wrong cases?

The "failure" here is because the direction is rtl and so the text direction of non-directional elements like '/' is wrong and they end up on the wrong side.

I don't know if it's possible there are consumers that display RTL-mixed-with-non-directional-or-LTR text that depend on the direction in the tooltip being RTL that would no longer display their (within-RTL) non-directional characters (or LTR elements) correctly if we changed the direction from RTL to auto.

In any case, changing the direction: CSS for *all* tooltips is a risky change. It's easier and less risky to change things 1 tooltip at a time, but there might be a lot of tooltips to change. I don't know.

Comment 11

[magicp]

Attachment: tooltips test cases.png

I confirmed all of tooltips in xul files. Then grouping them and selected some tooltips for test cases. It works well and I hope it will help you. Please find attached file. Thanks.

Comment 12

[Neil Deakin]

Is this the case that the tooltips that display uris need to use direction: ltr ? (Or use the class uri-element)

Comment 13

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #12)
> Is this the case that the tooltips that display uris need to use direction:
> ltr ? (Or use the class uri-element)

Maybe. Not all of them only display a URI, though. It's possible the ones that display both a description and a URI would need to be refactored to use a tooltip element with an explicit <label> that we can give that class, or something...

Comment 14

[:Gijs (he/him)]

Attachment: Bug 1313751 - fix direction of URLs and/or page titles in a number of UI elements,

Review commit: https://reviewboard.mozilla.org/r/91460/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91460/

Comment 15

[:Gijs (he/him)]

I believe the attached patch fixes everything in attachment 8805921 [details] except for the URL bar tooltip, which is more work because it uses tooltiptext, and we'd need to update it to use a different tooltip element in order to be able to style it selectively. I don't think we can blanket-style all |tooltip| elements in Firefox with unicode-bidi: -moz-plaintext and hope for the best.

Comment 16

[:Gijs (he/him)]

Comment on attachment 8808713 [details]
Bug 1313751 - fix direction of URLs and/or page titles in a number of UI elements,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91460/diff/1-2/

Comment 17

[Neil Deakin]

Comment on attachment 8808713 [details]
Bug 1313751 - fix direction of URLs and/or page titles in a number of UI elements,

https://reviewboard.mozilla.org/r/91460/#review91294

::: browser/base/content/browser.css:626
(Diff revision 2)
>  }
>  
> +.bookmark-item > label {
> +  /* ensure we use the direction of the bookmarks label instead of the
> +   * browser locale */
> +  unicode-bidi: -moz-plaintext;

The -moz prefix shouldn't be needed.

Comment 18

[:Gijs (he/him)]

Comment on attachment 8808713 [details]
Bug 1313751 - fix direction of URLs and/or page titles in a number of UI elements,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/91460/diff/2-3/

Comment 19

[Neil Deakin]

Comment on attachment 8808713 [details]
Bug 1313751 - fix direction of URLs and/or page titles in a number of UI elements,

https://reviewboard.mozilla.org/r/91460/#review91750

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/948ab12433f4
fix direction of URLs and/or page titles in a number of UI elements, r=enndeakin

Comment 21

[:Gijs (he/him)]

Hrmpf, after reviewing the screenshot one more time, I think I'm also not fixing the bhTooltip's main label/title/thing with this patch. Rebuilding to confirm, if I'm right I'll file another followup.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/948ab12433f4

Comment 23

[Forhad Hossain]

I have reproduced the bug in Nightly 52.0a1 (2016-10-28) with the instruction from comment 0 and Linux 64 Bit

Bug is fixed now on Latest Beta 53.0b3 (Build ID:20170316115347)
User Agent:Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0

[testday-20170317]

Comment 24

[:Gijs (he/him)]

Thanks!