473440 - LTR edit box context menu appears LTR in RTL locales

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[(no longer active)]

Attachment: Test case (to be loaded in RTL interface)

STR:

1. Load an RTL Firefox (he, fa or ar) or use Force RTL <https://addons.mozilla.org/en-US/firefox/addon/7438> to make the UI RTL.
2. Load the attached test case.
3. Right click on the text box.

Expected: The context menu should appear as RTL.

Result: The context menu appears LTR.

This is most prominently observed in the location bar for RTL locales (which set the location bar to be LTR).

The fix probably is to make sure the direction of the edit box's context menu comes from the direction of the parent of the textbox, rather than the textbox itself.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Simple fix.  Make textbox context menus obey the global direction, not the textbox direction.

Comment 2

[Dão Gottwald [:dao]]

Hm, can we just fix the location bar so that we make the input rather than the input-box ltr?

Comment 3

[Dão Gottwald [:dao]]

I guess the more relevant question is: Are there other cases where doing this in toolkit would be helpful?

Comment 4

[(no longer active)]

(In reply to comment #3)
> I guess the more relevant question is: Are there other cases where doing this
> in toolkit would be helpful?

Yes.  Another quick example would be the Home page box in the main prefpane.  Taking a look around, commonDialog.xul has the same problem.  This really belongs in toolkit, since any toolkit-based app is potentially subject to this problem, and it is quite common in RTL UIs to have LTR text boxes (which contain mostly English data, or things like URLs, etc.)

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 356931 [details] [diff] [review]
Patch (v1)

please add a class name to the menupopup and use that in the css files.

Comment 6

[(no longer active)]

(In reply to comment #5)
> (From update of attachment 356931 [details] [diff] [review])
> please add a class name to the menupopup and use that in the css files.

You mean something like class="&locale.dir;"?  Or adding a class programatically by reading the computed style?

Can I ask why you're suggesting this?

Comment 7

[Dão Gottwald [:dao]]

I mean a class name like "textbox-contextmenu".

Comment 8

[(no longer active)]

Attachment: Patch (v2)

Ah, right.  Here you are.  :-)

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 356953 [details] [diff] [review]
Patch (v2)

>+      <xul:menupopup anonid="input-box-contextmenu" class="textbox-contextmenu" chromedir="&locale.dir;"

This line gets quite long, should wrap it.

> .textbox-input-box menupopup {
>   cursor: default;

Please also make this use the new class.

>+menupopup.textbox-contextmenu[chromedir="rtl"] {

No need for the "menupopup" part.

Comment 10

[Dão Gottwald [:dao]]

(In reply to comment #9)
> >+menupopup.textbox-contextmenu[chromedir="rtl"] {
> 
> No need for the "menupopup" part.

Oh, and this belongs in xul.css.

Comment 11

[(no longer active)]

Attachment: Patch (v2.1)

Addressed comments.

Comment 12

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/9750c237c3ff

Comment 13

[(no longer active)]

Comment on attachment 356954 [details] [diff] [review]
Patch (v2.1)

Simple and low-risk patch.

Comment 14

[Philip Chee]

--- a/toolkit/content/widgets/textbox.xml
+++ b/toolkit/content/widgets/textbox.xml
@@ -487,16 +487,18 @@
+                     class="textbox-contextmenu"
+                     chromedir="&locale.dir;"

Shouldn't you add something like the following to textbox.xml?

<!DOCTYPE bindings [
  <!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">
  %globalDTD;
]>

Instead of assuming that all consumers would include global.dtd?

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #14)
http://mxr.mozilla.org/mozilla-central/search?string=global.dtd&find=textbox.xml

Comment 16

[Philip Chee]

Aaagh, Thanks. I think I need my glasses changed.

Comment 17

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 356954 [details] [diff] [review]
Patch (v2.1)

a191=beltzner

Comment 18

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6b2b215629fd