Closed
Bug 978309
Opened 11 years ago
Closed 11 years ago
Don't show white arrows on bookmarks panel folders when active on Linux & Windows
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 30
People
(Reporter: Gijs, Assigned: Gijs)
References
Details
(Whiteboard: [Australis:P3])
Attachments
(3 files, 1 obsolete file)
11.98 KB,
image/png
|
Details | |
275 bytes,
image/png
|
Details | |
8.97 KB,
patch
|
mikedeboer
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [Australis:P-] → [Australis:P3]
Assignee | ||
Comment 1•11 years ago
|
||
Looks like this is completely controlled through native code. :-\
Jim, is that right? Is this glyph provided and colored through nsNativeTheme.cpp? That's what it looks like to me...
Flags: needinfo?(jmathies)
![]() |
||
Comment 2•11 years ago
|
||
I'm not sure. mbrubeck was working on these elements for highdpi and was having a hard time finding the code that rendered them. I haven't had a chance to look at this but can.
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #2)
> I'm not sure. mbrubeck was working on these elements for highdpi and was
> having a hard time finding the code that rendered them. I haven't had a
> chance to look at this but can.
I mean, I guess the end of the story will be that we'll need to override it (with an image) specifically for the bookmarks menu, so I can work from there. Unless you have time to do exactly that? :-)
![]() |
||
Comment 4•11 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Jim Mathies [:jimm] from comment #2)
> > I'm not sure. mbrubeck was working on these elements for highdpi and was
> > having a hard time finding the code that rendered them. I haven't had a
> > chance to look at this but can.
>
> I mean, I guess the end of the story will be that we'll need to override it
> (with an image) specifically for the bookmarks menu, so I can work from
> there. Unless you have time to do exactly that? :-)
I don't know, can you post a screen shot of what the issue is or str to reproduce? I have no context.
Flags: needinfo?(jmathies)
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #4)
> (In reply to :Gijs Kruitbosch from comment #3)
> > (In reply to Jim Mathies [:jimm] from comment #2)
> > > I'm not sure. mbrubeck was working on these elements for highdpi and was
> > > having a hard time finding the code that rendered them. I haven't had a
> > > chance to look at this but can.
> >
> > I mean, I guess the end of the story will be that we'll need to override it
> > (with an image) specifically for the bookmarks menu, so I can work from
> > there. Unless you have time to do exactly that? :-)
>
> I don't know, can you post a screen shot of what the issue is or str to
> reproduce? I have no context.
Sure. Here's a screenshot. Basically, the issue is that we use non-native styling and so the white arrow for submenus is invisible. It should be using the black one throughout. I suspect we'll just need to provide an image and use that for all versions of Windows. Might be able to reuse an image that's in the tree already.
Assignee | ||
Comment 6•11 years ago
|
||
Stephen, can you provide assets we can use here, probably for both Windows and Linux?
Flags: needinfo?(shorlander)
Comment 7•11 years ago
|
||
Is this what you are looking for : http://mxr.mozilla.org/mozilla-central/ident?i=NS_THEME_MENUARROW ?
Comment 8•11 years ago
|
||
Btw, this hack might work :
.subviewbutton .menu-right {
color: black !important;
}
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #7)
> Is this what you are looking for :
> http://mxr.mozilla.org/mozilla-central/ident?i=NS_THEME_MENUARROW ?
No, I know about that code. It uses native Windows code to render the arrow, and there's no real way to influence that without altering our widget code, which would be weird. We simply need .png images to replace these arrows (which are done through so -moz-appearance: menuarrow).
(In reply to Tim Nguyen [:ntim] from comment #8)
> Btw, this hack might work :
> .subviewbutton .menu-right {
> color: black !important;
> }
No, that does not work. Nor will anything else along those lines: the code for the color of the arrow doesn't look at CSS at all.
Comment 10•11 years ago
|
||
Based on the arrows in shorlanders mockup, but adjusted to meet the default Windows colors.
Flags: needinfo?(shorlander)
Assignee | ||
Comment 11•11 years ago
|
||
Alright, how is this, Mike? Tested on Win XP Luna and Win7 Aero.
Attachment #8388911 -
Flags: review?(mdeboer)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment 12•11 years ago
|
||
This needs to be done for Linux too, I'm afraid...
OS: Windows XP → All
Hardware: x86 → All
Updated•11 years ago
|
Attachment #8388911 -
Flags: review?(mdeboer)
Comment 13•11 years ago
|
||
Apart from that, re. code it looks a-ok to me!
Updated•11 years ago
|
Summary: [WinXP] Don't show white arrows on bookmarks panel folders when active → Don't show white arrows on bookmarks panel folders when active on Linux & Windows
Assignee | ||
Comment 14•11 years ago
|
||
Now with linux support. Needed a height/width setting because menu.css sets those, and otherwise the icon looks warped.
Attachment #8389185 -
Flags: review?(mdeboer)
Assignee | ||
Updated•11 years ago
|
Attachment #8388911 -
Attachment is obsolete: true
Comment 15•11 years ago
|
||
Comment on attachment 8389185 [details] [diff] [review]
add custom arrows for Australis bookmarks menu panel on Windows and Linux,
Review of attachment 8389185 [details] [diff] [review]:
-----------------------------------------------------------------
Since the two images are identical, can you place it in ../shared/places/bookmarks-menu-arrow.png and update the jar.mn entries?
I also was able to have off 33% using ImageOptim.app here...
The CSS looks good. Shame of the code duplication, but that can not be avoided.
Attachment #8389185 -
Flags: review?(mdeboer) → review-
Comment 16•11 years ago
|
||
Comment on attachment 8389185 [details] [diff] [review]
add custom arrows for Australis bookmarks menu panel on Windows and Linux,
Discussed this on IRC. r=me with a minified version of the arrows png.
Attachment #8389185 -
Flags: review- → review+
Assignee | ||
Comment 17•11 years ago
|
||
Whiteboard: [Australis:P3] → [Australis:P3][fixed-in-fx-team]
Comment 18•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3][fixed-in-fx-team] → [Australis:P3]
Target Milestone: --- → Firefox 30
Assignee | ||
Comment 19•11 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #14)
> Created attachment 8389185 [details] [diff] [review]
> add custom arrows for Australis bookmarks menu panel on Windows and Linux,
>
> Now with linux support. Needed a height/width setting because menu.css sets
> those, and otherwise the icon looks warped.
So this hunk disappeared. Sticking this in bug 969592 because I noticed it there.
Assignee | ||
Comment 20•11 years ago
|
||
Comment on attachment 8389185 [details] [diff] [review]
add custom arrows for Australis bookmarks menu panel on Windows and Linux,
[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis' bookmarks panel
User impact if declined: arrows are hard to see on Windows and Linux
Testing completed (on m-c, etc.): on m-c, manual
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #8389185 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
status-firefox29:
--- → affected
status-firefox30:
--- → fixed
Updated•11 years ago
|
Attachment #8389185 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 21•11 years ago
|
||
Assignee | ||
Comment 22•11 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #21)
> https://hg.mozilla.org/mozilla-central/rev/cc1fdefca201
This was really bug 981700.
Assignee | ||
Comment 23•11 years ago
|
||
Updated•11 years ago
|
QA Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•