Make uri-element class work on any element

RESOLVED FIXED in mozilla1.9beta5

Status

()

Toolkit
XUL Widgets
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: philor, Assigned: philor)

Tracking

(Blocks: 1 bug, {intl})

Trunk
mozilla1.9beta5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

11 years ago
Created attachment 287485 [details] [diff] [review]
Fix v.1

The "uri-element" class that bug 157607 added to xul.css currently only works for textboxes and menulists, which are most, but not all, of the things where we display URIs that ought to remain ltr in an rtl locale - bug 400882 adds an hbox around textbox+label that together make up a URI, bug 213727 wants to add a listbox of URIs, and there are probably more I haven't spotted.

Currently doesn't affect Firefox, which doesn't use uri-element, but might make it possible to clean up its rather confusing current state where it's per-theme, and who knows what you might get with a third-party theme.
Flags: in-testsuite-
Attachment #287485 - Flags: review?(mconnor)
(Assignee)

Comment 1

11 years ago
Created attachment 306933 [details] [diff] [review]
Fix v.2

Maybe this alternate approach will work better :)

Note that I don't give a rat's ass whether I get r+ or WONTFIX, but I've been blocked on making the account manager not look stupid in RTL for a few days shy of four months now waiting for a simple decision on this.
Attachment #287485 - Attachment is obsolete: true
Attachment #306933 - Flags: review?(mano)
Attachment #287485 - Flags: review?(mconnor)
(Assignee)

Comment 2

11 years ago
Created attachment 307156 [details] [diff] [review]
Really fix v.2

Apparently, if you notice you left a stray comma and take it out, then you need to diff *after* you do.
Attachment #306933 - Attachment is obsolete: true
Attachment #307156 - Flags: review?(mano)
Attachment #306933 - Flags: review?(mano)
Comment on attachment 307156 [details] [diff] [review]
Really fix v.2

PLEASE GET YOUR CHANGES REVIEWED BY mconnor@steelgryphon.com AS WELL.
Attachment #307156 - Flags: review?(mano) → review+
(Assignee)

Comment 4

10 years ago
Comment on attachment 307156 [details] [diff] [review]
Really fix v.2

SINCE mconnor@mozilla.com IGNORED MY REVIEW REQUEST FOR FOUR MONTHS, I DON'T SEE HOW THAT'S ANY DIFFERENT FROM WONTFIX.
Attachment #307156 - Flags: review?(mconnor)
Comment on attachment 307156 [details] [diff] [review]
Really fix v.2

Huh, so this is the bug you were talking about.  If I wasn't clearly lazy and mean I'd have emailed you instead of waiting for you to come back on IRC before I forgot again. :)

Was going to change this to Neil Deakin going forward, been meaning to do that for like three months, so instead of removing that, please just change to enndeakin@sympatico.ca rather than removing it and r=me.  The point of the requirement is that bad rules here can break XUL and cause performance issues, etc, so someone who knows XUL layout internals is best to own this...
Attachment #307156 - Flags: review?(mconnor) → review+
(Assignee)

Updated

10 years ago
Attachment #307156 - Flags: approval1.9?
(Assignee)

Updated

10 years ago
Attachment #307156 - Flags: approval1.9?
(Assignee)

Comment 6

10 years ago
Created attachment 308104 [details] [diff] [review]
For checkin

With requested change; thanks.

Drivers: zero direct risk to Firefox, which doesn't use this class. The only potential risk is to extensions which are using the same class on things other than textboxes or menulists, but since they would have to use the same classname with nothing else in the selector to not be more specific, to have this rule override their rule they would have to be applying their stylesheet *before* xul.css, which as far as I know isn't possible. Reward is limiting the proliferation of classes for directionality of URI bits and bobs, which will make life easier in the future when we have to deal with the issue of multiple text directions in a URI.
Attachment #308104 - Flags: approval1.9?
Comment on attachment 308104 [details] [diff] [review]
For checkin

a1.9+=damons
Attachment #308104 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 8

10 years ago
toolkit/content/xul.css 1.118
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
You need to log in before you can comment on or make changes to this bug.