Closed Bug 424286 Opened 17 years ago Closed 17 years ago

Use different icons for all special folders (all bookmarks, bookmark menu, bookmark sidebar, tags, history, unsorted bookmarks)

Categories

(Firefox :: Theme, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: faaborg, Assigned: asaf)

References

Details

Attachments

(14 files, 2 obsolete files)

128.08 KB, image/png
Details
37.52 KB, image/png
Details
939 bytes, image/png
Details
157.68 KB, image/png
Details
562 bytes, image/png
Details
612 bytes, image/png
Details
18.58 KB, patch
dietrich
: review+
Details | Diff | Splinter Review
38.50 KB, image/png
Details
15.17 KB, image/png
Details
21.31 KB, image/png
Details
4.54 KB, patch
mconnor
: review+
mconnor
: approval1.9+
Details | Diff | Splinter Review
8.33 KB, image/png
Details
748 bytes, image/png
beltzner
: ui-review+
Details
1.27 KB, patch
asaf
: review+
beltzner
: approval1.9+
Details | Diff | Splinter Review
The latest windows icon drop includes several new places icons: /browser/themes/winstripe/browser/places/bookmarksMenu.png /browser/themes/winstripe/browser/places/bookmarksToolbar.png /browser/themes/winstripe/browser/places/history.png /browser/themes/winstripe/browser/places/importAndBackup.png /browser/themes/winstripe/browser/places/organize.png /browser/themes/winstripe/browser/places/view.png I'll attach mockups of where these icons should be used, let me know if you want me to split this off into individual bugs. I'm cc'ing the linux and OS X themers since ideally we should use the same icon names across the different themes.
Flags: blocking-firefox3?
Here is a mockup of where to use the new places icons. This mockup also includes the use of some existing icons, like tag.png
Summary: Places Icons → Style the places organizer with icons
Target Milestone: --- → Firefox 3
Attached image Library window on OS X
Example of how these three files could appear on OS X without any accompanying text. /browser/themes/winstripe/browser/places/importAndBackup.png /browser/themes/winstripe/browser/places/organize.png /browser/themes/winstripe/browser/places/view.png
Summary: Style the places organizer with icons → Style the places organizer, bookmarks sidebar, and bookmarks menu with icons
Linux is already alright for the importAndBackup, organize and view images. Question though: what CSS would we need to separate the different types of folders or queries?
Alex, some time ago we had an agreement not to use icons for view, organize and importAndBackup.. did this change? Michael: if those icons are meant not to be removed on Linux, we will need new ones. None of the current stock icons does fit here. bookmarksMenu.png and bookmarksToolbar.png should be in by now, see bug 418868
Attached image history.png for linux
history.png is ready but not yet checked in
(In reply to comment #5) > history.png is ready but not yet checked in It's actually in gnomestripe as a part of Toolbar.png and Toolbar-small.png, so we can just reuse it there. No need to add a file with the same icon we already have and can use.
IMO I find that the stock icons fit in the functions very well (I was very careful to select the stock icons when I first themed the Organizer), and the fact that they are stock icons and thus change with the theme is a big plus.
(In reply to comment #7) > IMO I find that the stock icons fit in the functions very well (I was very > careful to select the stock icons when I first themed the Organizer), and the > fact that they are stock icons and thus change with the theme is a big plus. > Organize = gtk-properties: normally used for properties of a file Views = gtk-sort-descending: normally used for sorting Z to A Import and Backup = gtk-revert-to-saved-ltr: normally used to undo all changes since the last save I don't see how any of those icons fits in well. The gtk-properties (the easiest to replace...) is perhaps generic enough to not do much harm but the other two have a totally different meaning. The make things worse, "gtk-revert-to-saved-ltr" is also used for "import" where it fits even less. So, Alex, do we want icons there now?
(In reply to comment #8) > (In reply to comment #7) > > IMO I find that the stock icons fit in the functions very well (I was very > > careful to select the stock icons when I first themed the Organizer), and the > > fact that they are stock icons and thus change with the theme is a big plus. > > > > Organize = gtk-properties: normally used for properties of a file > Views = gtk-sort-descending: normally used for sorting Z to A > Import and Backup = gtk-revert-to-saved-ltr: normally used to undo all changes > since the last save > > I don't see how any of those icons fits in well. The gtk-properties (the > easiest to replace...) is perhaps generic enough to not do much harm but the > other two have a totally different meaning. gtk-sort-descending is to do with sorting, which is precisely what the views menu does. gtk-revert-to-saved is related to getting back what you had saved, one of the main functions of _Import_ and Backup. I don't think this anywhere near outweighs the benefits of stock icon use.
Problem is they are all stock _action_ icons so they where not meant as generic icons for roughly related tasks or as category icons.
Just a note that this is either a dup of the Windows Bug 420551, or that this has already been split off.
I have to agree with Michael Monreal. The current icons used feels a bit confusing because they are commonly used for completely different things. I was also under the impression that we wouldn't be using icons there on Linux.
Blocks: 414389, 424028
Please use CSS. Please do not Hard-coded chrome URI of icons. This is really difficult to do theming. example (good): #bookmarksToolbarFolderMenu treechildren::-moz-tree-image(title, container, bookmarksToolbar) treechildren::-moz-tree-image(title, container, bookmarksMenu) treechildren::-moz-tree-image(title, container, historyContainer) example (bad): <menu id="bookmarksToolbarFolderMenu" image="chrome://browser/skin/places/bookmarksToolbar.png"/> #define BOOKMARKS_MENU_ICON_URI "chrome://browser/skin/places/bookmarksMenu.png" #define BOOKMARKS_TOOLBAR_ICON_URI "chrome://browser/skin/places/bookmarksToolbar.png" #define HISTORY_CONTAINER_ICON_URI "chrome://browser/skin/places/history.png"
Are the icons for Bookmarks Toolbar, History, Bookmarks Menu, etc. hooked up? I can't find where to set them unless I am just overlooking it.
On Windows we should *not* be including the icon for the Bookmark Toolbar in the drop-down menu. We don't include any icons there for things that aren't bookmarks.
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
(In reply to comment #16) > On Windows we should *not* be including the icon for the Bookmark Toolbar in > the drop-down menu. We don't include any icons there for things that aren't > bookmarks. Uh, ignore comment 16. Consider it an involuntary spasm. We've had icons there for a while now, and nobody's died. My bad.
Alex, please drive this with the various themes.
Assignee: nobody → faaborg
>Alex, some time ago we had an agreement not to use icons for view, organize and >importAndBackup.. did this change? Deciding what icons you want to use (or if you want to use icons) for the toolbar buttons is totally your call, based on what you think makes the most sense given the surrounding platform. Similarly on OS X Stephen and Kevin can decide what icons make the most sense (as a suggestion icons out of the finder seem to fit well https://bugzilla.mozilla.org/attachment.cgi?id=310928) However for the icons in the tree view, side bar and menus, I think it would be best if all the platforms used reasonably consistent images.
>Are the icons for Bookmarks Toolbar, History, Bookmarks Menu, etc. hooked up? I >can't find where to set them unless I am just overlooking it. They are checked in on winstripe so we can use consistent file names with pinstripe and gnomestripe, however I don't think the files have been added to a chrome manifest yet on any platform.
(In reply to comment #20) > They are checked in on winstripe so we can use consistent file names with > pinstripe and gnomestripe, however I don't think the files have been added to a > chrome manifest yet on any platform. The icons are already in the chrome manifests for winstripe.
Attached image OS X Library Window
(In reply to comment #19) > However for the icons in the tree view, side bar and menus, I think it would be > best if all the platforms used reasonably consistent images. > Here is what I have currently implemented for OS X. I would say they are fairly consistent. Although I didn't put a full fledged book on the toolbar folder since the working area was so small. I thought a blue star on a toolbar worked well enough.
Blocks: 425582
Assignee: faaborg → mano
(In reply to comment #22) > Here is what I have currently implemented for OS X. Stephen, Were you able to apply those icons to the live Library (and if so how did you do it), or is that a mock-up?
Are the tree-cell properties available to select in CSS the cells for these icons in the Library sidebar and main browser sidebar? I spent some time looking through places.js and friends, but must have missed it if it's there.
Blocks: 405605
Depends on: 427555
Blocks: 424877
Stephen's organizer toolbar icons were checked in for Mac as part of bug 427555. The sidebar icons are also in there.
Use this icon for all bookmarks on XP
Use this icon for all bookmarks on Vista
Um, perhaps I should clarify that I meant to say the icon is for the single item "All Bookmarks" in the left pane, and not actually all of the bookmarks :)
It looks like dirlisting/remote.png and dirlisting/local.png are represented as data: URIs in nsIndexedToHTML.cpp: http://tinyurl.com/4x35c8 http://tinyurl.com/5xu7h5
Whoops wrong bug, meant Bug 424877.
No longer depends on: 424951
Depends on: 408660
Attached patch os x part (obsolete) — Splinter Review
Attachment #314433 - Flags: review?(dietrich)
Attached patch both (obsolete) — Splinter Review
Attachment #314433 - Attachment is obsolete: true
Attachment #314439 - Flags: review?(dietrich)
Attachment #314433 - Flags: review?(dietrich)
Do we need to update gnomestripe?
(In reply to comment #33) > Do we need to update gnomestripe? Yes.
Do these icons fit there (both folders and toolbar images?)?
Gnomestripe todo: - Icons for bookmarks menu/toolbar can be found here: https://bugzilla.mozilla.org/attachment.cgi?id=304998 - The history icon also needs to be replaced in gnomestripe (icon already in the Toolbar-small stripe) - organize, view, importAndBackup: as the tango artists all agree that there should be no icons I don't see anyone redoing those... So we need a patch to disable them.
Comment on attachment 314439 [details] [diff] [review] both removing review request. need gnomestripe changes, per previous comments.
Attachment #314439 - Flags: review?(dietrich)
Also I might have missed this when looking at the patch quickly, but are we giving live mark items in the right pane the livemark item icon? http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/livemark-item.png
> - organize, view, importAndBackup: as the tango artists all agree that there > should be no icons I don't see anyone redoing those... So we need a patch to > disable them. As in, should I remove the current rules? They seem to use relevant stock icons.
>They seem to use relevant stock icons. My understanding is that the stock icons are not semantically correct. Also, the icon used for import and backup doesn't really match any of the other stock icons (perhaps it wasn't updated in someone else's visual refresh). There has been some debate back and forth on what to do, but I'm going to side with the tango artists and ask that we remove the three stock icons.
(In reply to comment #40) > >They seem to use relevant stock icons. > > > My understanding is that the stock icons are not semantically correct. Also, > the icon used for import and backup doesn't really match any of the other stock > icons (perhaps it wasn't updated in someone else's visual refresh). There has > been some debate back and forth on what to do, but I'm going to side with the > tango artists and ask that we remove the three stock icons. > The stock icons are not semantically correct *in your theme*. In my theme (the much more common GNOME icon theme) the icon uses a sheet of paper instead of a floppy, has much nicer colours, and seems to fit things nicer. I'm not sure why we should change it when some people would expect that style of icon due to the use of their theme.
It would appear that the icons are correct in terms of meaning (comment #9), but not in terms of use (specific single action vs. a group of related action). I agree with monreal and Nilsson's position that we should remove the icons. Also this is basically a Vista UI, so removing the icons will make it feel a little more native on Linux, more like a menu and less like a toolbar-ish thing.
Attached patch patchSplinter Review
Attachment #314439 - Attachment is obsolete: true
Attachment #314453 - Flags: review?(dietrich)
Comment on attachment 314453 [details] [diff] [review] patch >- // For consistency, we always return a favicon for non-containers, >- // even if it is just the default one. >- var icon = node.icon || PlacesUtils.favicons.defaultFavicon; >- return icon ? icon.spec : ""; >+ var icon = node.icon; >+ if (icon) >+ return icon.spec; >+ return ""; > }, you removed the default favicon from regular items from all platforms here, but only re-added it via css to winstripe and pinstripe. was it intentional to leave out gnomestripe? r=me otherwise
Attachment #314453 - Flags: review?(dietrich) → review+
nm previous comment, it's there.
mozilla/browser/components/places/content/treeView.js 1.62 mozilla/browser/themes/gnomestripe/browser/browser.css, 1.206 mozilla/browser/themes/gnomestripe/browser/places/organizer.css 1.13 mozilla/browser/themes/gnomestripe/browser/places/places.css 1.31 mozilla/browser/themes/pinstripe/browser/browser.css 1.141 mozilla/browser/themes/pinstripe/browser/places/places.css 1.22 mozilla/browser/themes/winstripe/browser/browser.css 1.195 mozilla/browser/themes/winstripe/browser/jar.mn 1.86 mozilla/browser/themes/winstripe/browser/places/allBookmarks-aero.png initial revision: 1.1 mozilla/browser/themes/winstripe/browser/places/allBookmarks.png initial revision: 1.1 mozilla/browser/themes/winstripe/browser/places/organizer.css 1.8 mozilla/browser/themes/winstripe/browser/places/places.css 1.23
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
"Tags" folder icon is not changed ?
Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre The Bookmarks toolbar icon is not displayed in the bookmarks menu.
(In reply to comment #47) > "Tags" folder icon is not changed ? > I think it should be changed and the unfiled bookmarks too.
No longer depends on: 424953
Checked with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040904 Minefield/3.0pre and everything looks fine. So there are remaining issues on Windows and perhaps Linux?
Remaining issues on Windows/Linux: The "Recently Bookmarked" and "Recent Tags" menuitems are identical .bookmark-item[query="true"][container="true"] They do not have separate ID's so they can take different icons. My Aeon themes have separate icons for queries and tags. I would like to use them here to continue the metaphors already existing in the Library. You work on this bug is not done. These items need separate ID's.
Ditto the "most visited" menuitem.
Alex, shouldn't we reopen the bug until all issues are getting fixed?
Was there a ui-review for the patch?
Another issue: the History item in the Places Library left panel shares the same icon as queries. But History is not really a query. Themers should be able to skin these icons differently from each other. Nice icons in the default theme.
Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9pre) Gecko/2008040907 Minefield/3.0pre History and Queries icons are different to me in the left panel of the library window.
Re- Comment #57. You are so right. Sorry for that. Should we have another bug for ui cleanup? For example, the Most Visited item on the Bookmarks Toolbar is a folder icon. In the Places tree - sidebar and library - it is a query icon.
Same for me here.
> > For example, the Most Visited item on the Bookmarks Toolbar is a folder icon. > In the Places tree - sidebar and library - it is a query icon. > Thats Bug 428098.
Reopening for adding an icon for unsorted bookmarks, I'll try to get the icon soon.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Alex: note the missing "all bookmarks" icon on mac as well. Also, what should we style the Tags folder with?
Tags folder should be /browser/places/tag.png I emailed Stephen and Kevin about the all bookmarks folder for pinstripe.
Whiteboard: [eta on icons?]
Summary: Style the places organizer, bookmarks sidebar, and bookmarks menu with icons → Use different icons for all special folders (all bookmarks, bookmark menu, bookmark sidebar, tags, history, unsorted bookmarks)
Default theme, Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041206 Minefield/3.0pre - Build ID: 2008041206
Default theme, Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041206 Minefield/3.0pre - Build ID: 2008041206
Another inconsistency: Bookmarks > Recent Tags > [list of tags containers] The list of tag containers are query icons (see Attachment #315304 [details]) Library > Tags folder > tag containers The tag containers have tag icons ((see Attachment #315305 [details]) Shouldn't the containers under "Recent Tags" have tag icons?
>Shouldn't the containers under "Recent Tags" have tag icons? yeah, they should
please, see also bug 428648 for work on tag containers (and adding of dayContainer, hostContainer attributes) comment #66 is almost certainly on an old profile, different icons are due to the fact that tag patch actually applies only to new profiles (see bug 425161).
Icons for unsorted bookmarks are now in: http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/places/unsortedBookmarks.png http://mxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/places/unsortedBookmarks-aero.png Other things that still need to be updated are: Library window and sidebars: -use tag icon for "Tags" in the left pane -use live bookmark folder icon for live bookmarks Bookmark contextual dialog: -use correct icons for menu, toolbar and unsorted in the drop down I think that is everything, am I missing anything? Stephen: any update on the OS X places icons?
faaborg or monreal: does Linux have the unsorted bookmarks icon yet? I haven't been following nightlies closely lately... Any other icons still needed for the Linux theme?
Attached image OS X Places Icons
> Stephen: any update on the OS X places icons? > I believe we have everything. Except I think the Tags folder should be showing as a tag.
(In reply to comment #70) > faaborg or monreal: does Linux have the unsorted bookmarks icon yet? I haven't > been following nightlies closely lately... Not yet. > Any other icons still needed for the Linux theme? If we remove the importAndBackup, organize and view icons I think we are done. Besides that, the Error/Warning/Info icons are still not replaced with stpck icon usage, so they currently use the windows look.
Whiteboard: [eta on icons?]
Can we get a status update here?
Whiteboard: [needs status update]
(In reply to comment #73) > Can we get a status update here? > Once Bug #430693 is taken care of OS X should be complete in this area.
Oops, forgot to say that the patch and the icon are ready and should be checked in tonight.
Mano, as per comment 69, it looks like we need to: - hook up the unsorted bookmarks icon - fix up usage in library window and sidebars - fix up usage in add bookmark drop-down dialog
Whiteboard: [needs status update] → [needs patch]
We'd take a patch here, but the issues from comment 76 don't block on their own. This change is CSS only, and could even be taken on branch.
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Attached patch patchSplinter Review
Attachment #318901 - Flags: review?(mconnor)
Attachment #318901 - Flags: approval1.9?
Attachment #318901 - Flags: review?(mconnor)
Attachment #318901 - Flags: review+
Attachment #318901 - Flags: approval1.9?
Attachment #318901 - Flags: approval1.9+
mozilla/browser/themes/gnomestripe/browser/places/places.css 1.34; mozilla/browser/themes/pinstripe/browser/places/places.css 1.26 mozilla/browser/themes/winstripe/browser/places/places.css 1.26
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Attached image Screenshot
{Build ID: 2008050200} I feel that treechildren::-moz-tree-image(container, OrganizerQuery_Tags) not works...
Also, this should probably not be fixed until Linux gets the unsorted bookmarks icon. faaborg, any updates on that?
(In reply to comment #81) > Created an attachment (id=318992) [details] > Screenshot > > {Build ID: 2008050200} > I feel that treechildren::-moz-tree-image(container, OrganizerQuery_Tags) not > works... > most probably due to bug 431817
Depends on: 431817
My take on the still missing "unfiled bookmarks" icon for linux.
Attached patch Tango patchSplinter Review
Integrates that icon into Gnomestripe.
Attachment #319253 - Flags: review?(mano)
For tracking purposes please let us reopen this bug until the remaining work for Linux is done.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #319253 - Flags: review?(mano)
Attachment #319253 - Flags: review+
Attachment #319253 - Flags: approval1.9?
Attachment #319251 - Flags: ui-review?(faaborg)
Attachment #319251 - Flags: ui-review?(faaborg) → ui-review+
Comment on attachment 319253 [details] [diff] [review] Tango patch a1.9=beltzner
Attachment #319253 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Alex, the folder drop down inside the bookmarks contextual dialog doesn't support the new icons at all. Shall I file a new bug about that issue or is there no intention to show special icons at this place?
>Alex, the folder drop down inside the bookmarks contextual dialog doesn't >support the new icons at all. Shall I file a new bug about that issue or is >there no intention to show special icons at this place? let's file a follow up bug on that. Ideally the three special folders in that drop down (menu, toolbar, and unsorted) would get the right icons.
If winstripe and pinstripe were updated on 5/1, why do we still have the smart folder for Tags in the 2008050506 builds?
(In reply to comment #90) > let's file a follow up bug on that. Ideally the three special folders in that > drop down (menu, toolbar, and unsorted) would get the right icons. Filed bug 432409. > If winstripe and pinstripe were updated on 5/1, why do we still have the smart > folder for Tags in the 2008050506 builds? You mean "Smart Bookmarks"? It is not automatically removed and has to be done manually. Only beta users will be affected from this folder.
Checking in browser/themes/gnomestripe/browser/jar.mn; /cvsroot/mozilla/browser/themes/gnomestripe/browser/jar.mn,v <-- jar.mn new revision: 1.79; previous revision: 1.78 done Checking in browser/themes/gnomestripe/browser/places/places.css; /cvsroot/mozilla/browser/themes/gnomestripe/browser/places/places.css,v <-- places.css new revision: 1.35; previous revision: 1.34 done RCS file: /cvsroot/mozilla/browser/themes/gnomestripe/browser/places/unsortedBookmarks.png,v done Checking in browser/themes/gnomestripe/browser/places/unsortedBookmarks.png; /cvsroot/mozilla/browser/themes/gnomestripe/browser/places/unsortedBookmarks.png,v <-- unsortedBookmarks.png initial revision: 1.1 done
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs patch]
Depends on: 432832
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: