Closed
Bug 969904
Opened 10 years ago
Closed 10 years ago
Update Bookmarks Star Icons for Consistency
Categories
(Firefox :: Bookmarks & History, defect)
Tracking
()
VERIFIED
FIXED
Firefox 30
People
(Reporter: fx4waldi, Assigned: shorlander)
References
Details
(Whiteboard: [Australis:P4+])
Attachments
(1 file, 3 obsolete files)
563.08 KB,
patch
|
mikedeboer
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
In some systems, the blue star does not look good, and is not consistent with the theme. I think it would look much better if animated star it was yellow, like the starred48
Comment 2•10 years ago
|
||
I discussed this with madhava and stephen some days ago, the blue star in the animation is expected across platforms, what's wrong is the yellow star in some themes. Though I don't know if there's already a bug to change those graphics to the new version.
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #2) > I discussed this with madhava and stephen some days ago, the blue star in > the animation is expected across platforms, what's wrong is the yellow star > in some themes. Though I don't know if there's already a bug to change those > graphics to the new version. There is not AFAIK. We can just use this one.
Assignee: nobody → shorlander
Blocks: australis-navbar
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(shorlander)
Summary: bookmarks-notification-finish should not be blue for some systems → Update Bookmarks Star Icons for Consistency
Assignee | ||
Comment 6•10 years ago
|
||
Patch updates Toolbar.png and menuPanel.png with new bookmark icons. Also updates bookmarks-notification-finish.png to more accurately match new star icons.
Attachment #8389434 -
Flags: review?(mdeboer)
Assignee | ||
Comment 7•10 years ago
|
||
Also had to update the coordinates to show the right icon in the Menu Panel.
Comment 8•10 years ago
|
||
Stephen, what about star-icons.png and .*starred48.*\.png? Are those being handled elsewhere?
Comment 9•10 years ago
|
||
and bookmark.png in the windows theme (we should probably try to unify all of these star images into fewer files)
Comment 10•10 years ago
|
||
You should make sure this doesn't conflict with the new sidebar widget (bug 960198)
Updated•10 years ago
|
Status: NEW → ASSIGNED
Flags: needinfo?(shorlander)
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Marco Bonardo [:mak] from comment #8) > Stephen, what about star-icons.png and .*starred48.*\.png? > Are those being handled elsewhere? I was going to file a follow-up and just get the stuff that's in primary UI updated. But I guess it makes sense to just go ahead and do it here :) (In reply to Marco Bonardo [:mak] from comment #9) > and bookmark.png in the windows theme (we should probably try to unify all > of these star images into fewer files) Ugh, thanks, forgot these. (In reply to Tim Nguyen [:ntim] from comment #10) > You should make sure this doesn't conflict with the new sidebar widget (bug > 960198) Thanks! Didn't realize that relanded.
Flags: needinfo?(shorlander)
Assignee | ||
Updated•10 years ago
|
Attachment #8389434 -
Flags: review?(mdeboer)
Comment 12•10 years ago
|
||
You might want to wait for bug 978483 to be fixed first.
Assignee | ||
Comment 13•10 years ago
|
||
Updated starred48.png for all platforms and added starred48@2x.png for OS X.
Attachment #8389434 -
Attachment is obsolete: true
Attachment #8390210 -
Flags: review?(mdeboer)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8390210 [details] [diff] [review] update-bookmark-star-states.patch Forgot to add the other stars again…
Attachment #8390210 -
Attachment is obsolete: true
Attachment #8390210 -
Flags: review?(mdeboer)
Assignee | ||
Comment 15•10 years ago
|
||
Think I got them all this time. For real.
Attachment #8390461 -
Flags: review?(mdeboer)
Assignee | ||
Updated•10 years ago
|
Whiteboard: [Australis:P5] → [Australis:P4+]
Comment 16•10 years ago
|
||
Comment on attachment 8390461 [details] [diff] [review] update-bookmark-star-states.patch Review of attachment 8390461 [details] [diff] [review]: ----------------------------------------------------------------- I know a new patch is coming up with an inverted Find icon for the Windows theme. - all the assets are nicely compressed and small. - various other improvements are made to the toolbar and menu-panel glyphs, which makes this a big win overall in contrast and sharper edges. - bookmark stars are nice & blue, across all platforms. So apart from the small CSS tweak, this should be ready to go! ::: browser/themes/windows/browser.css @@ +1314,5 @@ > } > > +richlistitem[selected="true"][current="true"] > .ac-title-box > .ac-result-type-bookmark, > +.autocomplete-treebody::-moz-tree-image(selected, current, bookmark, treecolAutoCompleteImage) { > + list-style-image: url("chrome://browser/skin/places/bookmark.png"); Can you move this rule just below the previous one that targets bookmarks? Also, I think you can remove this list-style-image property, because it'll already be set and correct; only the coordinates are needed.
Attachment #8390461 -
Flags: review?(mdeboer) → feedback+
Assignee | ||
Comment 17•10 years ago
|
||
Updated patch to fix flipped menuPanel find icons on Windows. Also fixed the selected list item rule.
Attachment #8390461 -
Attachment is obsolete: true
Attachment #8390491 -
Flags: review?(mdeboer)
Comment 18•10 years ago
|
||
Comment on attachment 8390491 [details] [diff] [review] update-bookmark-star-states.patch Review of attachment 8390491 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! I'll push this big update for you.
Attachment #8390491 -
Flags: review?(mdeboer) → review+
Comment 19•10 years ago
|
||
Landed as: https://hg.mozilla.org/integration/fx-team/rev/02be46ac2b55
status-firefox29:
--- → affected
status-firefox30:
--- → affected
Whiteboard: [Australis:P4+] → [Australis:P4+][fixed-in-fx-team]
Comment 20•10 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #19) > Landed as: https://hg.mozilla.org/integration/fx-team/rev/02be46ac2b55 I see the patch adds a menuPanel-aero.png , but not sure if it's used anywhere in the CSS. I'm guessing the menuPanel.png is for Windows 8+ and menuPanel-aero.png for Windows 7, Vista and XP.
Flags: needinfo?(mdeboer)
Comment 21•10 years ago
|
||
(In reply to Tim Nguyen [:ntim] from comment #20) > (In reply to Mike de Boer [:mikedeboer] from comment #19) > > Landed as: https://hg.mozilla.org/integration/fx-team/rev/02be46ac2b55 > > I see the patch adds a menuPanel-aero.png , but not sure if it's used > anywhere in the CSS. > I'm guessing the menuPanel.png is for Windows 8+ and menuPanel-aero.png for > Windows 7, Vista and XP. Nevermind, ignore the comment.
Flags: needinfo?(mdeboer)
https://hg.mozilla.org/mozilla-central/rev/02be46ac2b55
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P4+][fixed-in-fx-team] → [Australis:P4+]
Target Milestone: --- → Firefox 30
Updated•10 years ago
|
Comment 23•10 years ago
|
||
Comment on attachment 8390491 [details] [diff] [review] update-bookmark-star-states.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): Australis User impact if declined: odd set of bookmarks icons Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): low, just an asset swap String or IDL/UUID changes made by this patch: none
Attachment #8390491 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8390491 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 24•10 years ago
|
||
The "Unsorted Bookmarks"-icons in win and linux also have a yellow star.
Comment 26•10 years ago
|
||
Verified as fixed on Firefox 29 RC and latest Aurora using Windows 7 64bit, Ubuntu 14.04 32bit and Mac OS X 10.9.2.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in
before you can comment on or make changes to this bug.
Description
•