Closed Bug 478430 Opened 16 years ago Closed 16 years ago

[RTL] URLs in exceptions dialogs are right-to-left

Categories

(Firefox :: Settings UI, defect)

Product:
Firefox
Component:
Settings UI
Version:
3.5 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3.6a1

People

(Reporter: whimboo, Assigned: ehsan.akhgari)

References

Details

(Keywords: fixed1.9.1, rtl)

Attachments

(2 files, 1 obsolete file)

URLs in password exceptions dialog
(deleted), image/jpeg
Details
Patch (v2)
(deleted), patch
Gavin
: review+
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
Attached image URLs in password exceptions dialog (deleted) —
The URLs in all the exception dialogs (images, cookies, passwords, ...) inside the preferences are aligned right-to-left. To have a distinctive look and feel shouldn't they also be aligned left-to-right? See the attached screenshot.
Dao, Enn: This bug may also benefit from the rule in bug 477902 being added to xul.css...
Depends on: 478377
Status: NEW → ASSIGNED
Depends on: 477902
This bug should fix the direction (to be always LTR), not the alignment.
Summary: [RTL] URLs in exceptions dialogs are aligned right-to-left → [RTL] URLs in exceptions dialogs are right-to-left
Attached patch Patch (v1) (obsolete) (deleted) — — Splinter Review
Attachment #364546 - Flags: review?(gavin.sharp)
Comment on attachment 364546 [details] [diff] [review] Patch (v1) Doesn't this need to add an atom? Does it even work?
Comment on attachment 364546 [details] [diff] [review] Patch (v1) Oh, I just saw bug 480581... Does xpconnect really know how to convert that string into an nsISupports* that points to an nsIAtom? That seems unlikely...
(In reply to comment #5) > (From update of attachment 364546 [details] [diff] [review]) > Oh, I just saw bug 480581... Does xpconnect really know how to convert that > string into an nsISupports* that points to an nsIAtom? That seems unlikely... It's true! See bug 336684.
Comment on attachment 364546 [details] [diff] [review] Patch (v1) Gavin is right. Because most host names don't change their appearance in RTL mode, I was fooled that this works, but after more careful testing, it does not work. I'll post a new patch here.
Attachment #364546 - Flags: review?(gavin.sharp)
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 364546 [details] [diff] [review] [details]) > > Oh, I just saw bug 480581... Does xpconnect really know how to convert that > > string into an nsISupports* that points to an nsIAtom? That seems unlikely... > > It's true! See bug 336684. Right, but in this case the param isn't nsIAtom*, it's nsISupports* (nsICollection::AppendElement).
Attached patch Patch (v2) (deleted) — — Splinter Review
Updated patch to create nsIAtom's explicitly.
Attachment #364546 - Attachment is obsolete: true
Attachment #364767 - Flags: review?(gavin.sharp)
Comment on attachment 364767 [details] [diff] [review] Patch (v2) >diff --git a/toolkit/components/passwordmgr/content/passwordManager.js b/toolkit/components/passwordmgr/content/passwordManager.js >diff --git a/toolkit/components/passwordmgr/content/passwordManagerExceptions.js b/toolkit/components/passwordmgr/content/passwordManagerExceptions.js You can put the kLTRAtom definition in passwordManagerCommon's Startup() instead of duplicating it in each of these files. r=me with that change, and a confirmation that you've tested that all of these changes are effective (because I haven't).
Attachment #364767 - Flags: review?(gavin.sharp) → review+
I have tested the password manager part using a host name which included a realm string (which is wrapped in parenthesis, so that the incorrect RTL direction can be observed), and the preferences window part by setting breakpoints and stepping into the core layout code.
http://hg.mozilla.org/mozilla-central/rev/a6522d930dd0
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-litmus?
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Comment on attachment 364767 [details] [diff] [review] Patch (v2) Nice to have for branch (for RTL locales)
Attachment #364767 - Flags: approval1.9.1?
Ehsan, can't I verify this with your add-on? The URL's are still rtl.
Are you talking about direction or alignment? The URLs should still be right-aligned. In order to verify their direction, you need a URL which ends in a weak bidi character, like a ")". See comment 11 for how to do that (for password manager at least).
I wondered because in the details pane of the library they are left aligned. Looks like that trees and lists have another behavior. Ehsan, do you have such an URL? It will help me in verifying.
Comment on attachment 364767 [details] [diff] [review] Patch (v2) a191=beltzner
Attachment #364767 - Flags: approval1.9.1? → approval1.9.1+
Ehsan, entering the URL into the textbox of one of the exception dialogs shows it right aligned. Given by your comment they should be left-aligned which will be the same behavior as what can be seen in the URL textbox of the Library details pane.
(In reply to comment #20) > Ehsan, entering the URL into the textbox of one of the exception dialogs shows > it right aligned. Given by your comment they should be left-aligned which will > be the same behavior as what can be seen in the URL textbox of the Library > details pane. No, we keep the LTR cells in a tree right-aligned. (See bug 477902 comment 28 for reference.)
(In reply to comment #21) > No, we keep the LTR cells in a tree right-aligned. (See bug 477902 comment 28 > for reference.) I talk about the filtering textbox and not the tree.
(In reply to comment #22) > I talk about the filtering textbox and not the tree. Ah, sorry I didn't notice. This bug doesn't touch the filter textbox. FWIW I don't think we should make the filter textbox LTR because it's not specific to entering URLs...q
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: