Closed
Bug 473440
Opened 16 years ago
Closed 16 years ago
LTR edit box context menu appears LTR in RTL locales
Categories
(Toolkit :: XUL Widgets, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
Details
(4 keywords)
Attachments
(2 files, 2 obsolete files)
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
(deleted),
patch
|
dao
:
review+
enndeakin
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•16 years ago
|
||
Simple fix. Make textbox context menus obey the global direction, not the textbox direction.
Comment 2•16 years ago
|
||
Hm, can we just fix the location bar so that we make the input rather than the input-box ltr?
Comment 3•16 years ago
|
||
I guess the more relevant question is: Are there other cases where doing this in toolkit would be helpful?
Assignee | ||
Comment 4•16 years ago
|
||
(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•16 years ago
|
||
Comment on attachment 356931 [details] [diff] [review]
Patch (v1)
please add a class name to the menupopup and use that in the css files.
Assignee | ||
Comment 6•16 years ago
|
||
(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•16 years ago
|
||
I mean a class name like "textbox-contextmenu".
Assignee | ||
Comment 8•16 years ago
|
||
Ah, right. Here you are. :-)
Attachment #356931 -
Attachment is obsolete: true
Attachment #356953 -
Flags: review?(dao)
Attachment #356931 -
Flags: review?(dao)
Comment 9•16 years ago
|
||
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•16 years ago
|
||
(In reply to comment #9)
> >+menupopup.textbox-contextmenu[chromedir="rtl"] {
>
> No need for the "menupopup" part.
Oh, and this belongs in xul.css.
Assignee | ||
Comment 11•16 years ago
|
||
Addressed comments.
Attachment #356953 -
Attachment is obsolete: true
Attachment #356954 -
Flags: review?(dao)
Attachment #356953 -
Flags: review?(dao)
Updated•16 years ago
|
Attachment #356954 -
Flags: review?(enndeakin)
Attachment #356954 -
Flags: review?(dao)
Attachment #356954 -
Flags: review+
Updated•16 years ago
|
Attachment #356954 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 12•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Comment 13•16 years ago
|
||
Comment on attachment 356954 [details] [diff] [review]
Patch (v2.1)
Simple and low-risk patch.
Attachment #356954 -
Flags: approval1.9.1?
Assignee | ||
Updated•16 years ago
|
Flags: in-litmus?
Comment 14•16 years ago
|
||
--- 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•16 years ago
|
||
Comment 16•16 years ago
|
||
Aaagh, Thanks. I think I need my glasses changed.
Comment 17•16 years ago
|
||
Comment on attachment 356954 [details] [diff] [review]
Patch (v2.1)
a191=beltzner
Attachment #356954 -
Flags: approval1.9.1? → approval1.9.1+
Assignee | ||
Comment 18•16 years ago
|
||
Keywords: fixed1.9.1
Assignee | ||
Updated•12 years ago
|
Flags: in-litmus?
You need to log in
before you can comment on or make changes to this bug.
Description
•