Closed
Bug 431057
Opened 17 years ago
Closed 17 years ago
RTL icon needed for the new tree twisty icons for the Aero theme
Categories
(Toolkit :: XUL Widgets, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
(Keywords: rtl)
Attachments
(1 file, 6 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
We need RTL versions of the new tree twisty icons, otherwise RTL trees look ridiculous. Alex said he's going to provide the new icons.
Comment 1•17 years ago
|
||
Here are all if the tree icons, including RTL states for the 4 aero icons.
Comment 2•17 years ago
|
||
I'm requesting blocking since having expansion arrows pointing in the wrong direction is pretty obviously broken.
Flags: blocking-firefox3?
Comment 3•17 years ago
|
||
How are we going to use the icons in CSS? chromedir isn't globally available.
Component: Theme → XUL Widgets
Flags: blocking-firefox3?
Product: Firefox → Toolkit
QA Contact: theme → xul.widgets
Updated•17 years ago
|
Flags: blocking1.9?
Comment 4•17 years ago
|
||
(In reply to comment #3)
> How are we going to use the icons in CSS? chromedir isn't globally available.
You mean unavailable from /toolkit? That's, uh, hrm. Can we override that by adding our own tree.css in /browser? I guess that would require moving the images into /browser as well and restoring the old non-RTL ones here.
Flags: blocking1.9? → blocking1.9+
(In reply to comment #4)
> (In reply to comment #3)
> > How are we going to use the icons in CSS? chromedir isn't globally available.
>
> You mean unavailable from /toolkit? That's, uh, hrm. Can we override that by
> adding our own tree.css in /browser? I guess that would require moving the
> images into /browser as well and restoring the old non-RTL ones here.
>
I think moving that out of toolkit would be a good idea, especially if the suggestion in bug 430852 comment 25 is ever implemented...
Comment 6•17 years ago
|
||
global.dtd is in dom/locales, so we can use it from toolkit without a problem. You can set chromedir on trees by setting it on the binding's <content>, too.
Assignee | ||
Comment 7•17 years ago
|
||
Thanks for the tip Gavin! Patch coming up shortly.
Assignee: nobody → ehsan.akhgari
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•17 years ago
|
||
Actually, this bug seems Vista specific, unless we want to add the code for all of the platforms and use identical RTL images for the themes which use symmetric (RTL-agnostic) tree twisty icons.
OS: All → Windows Vista
Hardware: All → PC
Assignee | ||
Comment 9•17 years ago
|
||
This patch should land with the icons in attachment 318328 [details].
Attachment #318713 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•17 years ago
|
Attachment #318328 -
Flags: approval1.9?
Comment 10•17 years ago
|
||
Comment on attachment 318328 [details]
New set of tree icons
a1.9+=damons
Attachment #318328 -
Flags: approval1.9? → approval1.9+
Comment 11•17 years ago
|
||
We are changing the file names from aero-rtl to rtl-aero for consistency, sorry about getting it wrong in the earlier icon set.
Comment 12•17 years ago
|
||
Here is an updated set of images. Note that the file names have been changed for rtl to appear before -aero instead of after. If bug 431633 is resolved then these icons will already be checked in, otherwise you can check them in with the patch on this bug.
Attachment #318328 -
Attachment is obsolete: true
Assignee | ||
Comment 13•17 years ago
|
||
Here's the updated patch considering the file name change.
Attachment #318713 -
Attachment is obsolete: true
Attachment #318758 -
Flags: review?(gavin.sharp)
Attachment #318713 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 14•17 years ago
|
||
Updated patch to handle the hover RTL icons as well. I also included the fix to bug 430895 in this patch (in tree-aero.css).
Attachment #318758 -
Attachment is obsolete: true
Attachment #318763 -
Flags: review?(gavin.sharp)
Attachment #318758 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 15•17 years ago
|
||
Comment on attachment 318757 [details]
Updated tree widget images
Now that bug 431633 has landed, this should no longer be necessary here.
Attachment #318757 -
Attachment is obsolete: true
Comment 16•17 years ago
|
||
Comment on attachment 318763 [details] [diff] [review]
Patch (v1.2)
>Index: toolkit/themes/winstripe/global/jar.mn
>+ skin/classic/aero/global/tree/twisty-clsd-rtl.png (tree/twisty-clsd-rtl-aero.png)
>+ skin/classic/aero/global/tree/twisty-clsd-hover-rtl.png (tree/twisty-clsd-hover-rtl-aero.png)
>+ skin/classic/aero/global/tree/twisty-open-rtl.png (tree/twisty-open-rtl-aero.png)
>+ skin/classic/aero/global/tree/twisty-open-hover-rtl.png (tree/twisty-open-hover-rtl-aero.png)
Looks like these changes have already landed.
>Index: toolkit/themes/winstripe/global/tree-aero.css
>+ Please note that the following RTL icons are only available in Aero themes:
The hover icons are also only in aero themes, so this comment should probably be at the top of the file and apply to all images referred to in this file.
Attachment #318763 -
Flags: review?(gavin.sharp) → review+
Comment 17•17 years ago
|
||
Comment on attachment 318763 [details] [diff] [review]
Patch (v1.2)
a1.9=beltzner
Attachment #318763 -
Flags: approval1.9+
Updated•17 years ago
|
Whiteboard: [has patch][has review][has approval]
Assignee | ||
Comment 18•17 years ago
|
||
This patch addresses Gavin's review comments, so I'm carrying forward r+ and a+ on this patch. This is the patch to land for this bug.
Attachment #318763 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•17 years ago
|
Summary: RTL icon needed for the new tree twisty icons → RTL icon needed for the new tree twisty icons for the Aero theme
Comment 19•17 years ago
|
||
For some reason I couldn't apply that previous patch cleanly (I got: patch: **** malformed patch at line 119), so I applied it manually.
mozilla/toolkit/content/widgets/tree.xml 1.57
mozilla/toolkit/themes/winstripe/global/jar.mn 1.58
mozilla/toolkit/themes/winstripe/global/tree-aero.css 1.1
Attachment #318865 -
Attachment is obsolete: true
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has review][has approval]
Target Milestone: --- → mozilla1.9
You need to log in
before you can comment on or make changes to this bug.
Description
•