DESCRIPTION: The menus created on the personal toolbar when the personal toolbar folder in the bookmarks has subfolders look wrong. In the default bookmarks file, I believe the last item in the personal toolbar folder has a channels folder. There are two problems: * The buttons for the folders (i.e., the "Channels" button) don't get a border when they are hovered over. * The items within the folders should have hover effects that look like the buttons outside the folders, not like items on a menubar. That is, they should have the icon change and probably a border, but should not have a blue stripe cover them. STEPS TO REPRODUCE: * Start apprunner with default bookmarks (if the defaults are still the same as they were a while back...) * Hover over channels button at left edge of personal toolbar * Click on that button and hover over the items in it DOES NOT WORK CORRECTLY ON: * Linux, apprunner, 1999-10-04-14-M11 WORKS CORRECTLY ON: * NN 4.61 Linux (except for borders on items within the menus)

The above patch (to navigator.css) fixes this cosmetic bug with the drop-down menus on the personal toolbar, except for the one touch that I couldn't get - having the folder icon open when the menu is active. I don't know if there's an attribute on the menus that's set by default or whether it has to be done in js. cc:ing folks based on cvs blame: matt because he's responsible for most of the file, and waterson because he wrote this part of the file. This bug is basically lying unclaimed in the Browser-General queue.

BTW, this patch isn't going to work against a current tree, since the image paths have changed since I last pulled. If you want to use my patch, you basically have to do it by hand (not that it's that hard)...

See bug 16651 for repainting problems that already exist, but are made more frequent by this fix (that is, they occur on submenu hovers in addition to hovers on the main personal toolbar).

I'm working on fixing the remaining problems. I fixed the one problem described above, but I'm still seeing some problems with nested submenus - I'll figure those out sometime and attach a new patch.

I tested this patch out with a submenu within a submenu - and decided to omit the hover effect on the submenu menu since the menu opens automatically. This patch also differs from the previous one in that it changes the folder icon when the menu is open (the remaining piece I mentioned before). It's also against a tree from yesterday evening, so it should be usable with patch.

matt: i've tried these changes in my tree; they look great. if you'll code review, I'll check them in.

looks good. Code reviewed.

checked in. thanks dbaron!

Here is an additional rule which improves the menu a bit more: toolbar.main-bar > menu[open="true"] > titledbutton { border: 1px white inset; }

There should probably be an equivalent rule for sub-submenus too... possibly with selector toolbar.main-bar menupopup > menu[open="true"] but it would also need the equivalent 1px solid transparent rule without the [open="true"] to prevent reflows.

I would actually prefer if the hover style for menu items on this menu matched the hover style for other menus (ie. mauve colour). Is there a reason why we are using white outset instead?

matt, can you take a look at these changes?

Above is a patch that makes the described changes, tested on 10-21 release build.

Clearing FIXED resolution due to reopen.

The skin has change so this is no longer valid

Actually, the problem is still there but the current attached patches are invalid. Fix is still needed to add hover effect to "Channels" menu, plus add open menu effect.

German: do you want to help me nail this one?

Moving to M12.

Here are new style rules which fix this bug. * To navigator.css, change: titledbutton.bookmarkbutton:hover { color: #CCFFFF; } * To... titledbutton.bookmarkbutton:hover, toolbar#PersonalToolbar menu:hover > titledbutton.bookmarkFolder { color: #CCFFFF; } * and, add this rule: toolbar#PersonalToolbar menu[open="true"] > titledbutton.bookmarkFolder { border: 1px #99CCCC inset; } * and in file "navigator.xul", change: <titledbutton class="bookmarkbutton" align="left" value="&bookmarksButton.label;" crop="right"/> * to: <titledbutton class="bookmarkFolder" align="left" value="&bookmarksButton.label;" crop="right"/>

i have the new changes in my tree if anyone want to code review Index: navigator.css =================================================================== RCS file: /cvsroot/mozilla/xpfe/browser/resources/skin/navigator.css,v retrieving revision 1.22 diff -c -r1.22 navigator.css *** navigator.css 1999/11/02 01:35:42 1.22 --- navigator.css 1999/11/03 02:20:23 *************** *** 134,141 **** background-image: none; } ! titledbutton.bookmarkbutton:hover { color: #CCFFFF; } titledbutton.bookmarkbutton:active { --- 134,146 ---- background-image: none; } ! titledbutton.bookmarkbutton:hover, ! toolbar#PersonalToolbar menu:hover > titledbutton.bookmarkFolder { color: #CCFFFF; + } + + toolbar#PersonalToolbar menu[open="true"] > titledbutton.bookmarkFolder { + border: 1px #99CCC inset; } titledbutton.bookmarkbutton:active {

The plan is that folder buttons on the personal bookmarks bar feature a small white triangle to thir left, in order to save space and clutter we will NOT have them show a folder icon. You accomplish by assigning class="popup" to them. The other attributes that you guys suggested for class="Bookmarkfolder" look right to me. Although I was specifically trying to avoid setting borders in order to save 2px of vertical space. So in order to get both your changes and the popup triangle use class="bookmarkFolder popup"

Please use borders. Things don't look like buttons without borders. (That's both a specific comment on this bug and a general criticism of the new UI.)

Reasons for not using borders are: - conserving space and reducing visual clutter - The buttons behave like links (except for folders)

As it currently stands, the personal toolbar can't be distinguished from a border around the edge of the window. It should be visible.

Bumping to M13 when German and I hope to have time to look at this more closely.

Moving to accurate component since I made the mistake of originally filing on Browser-General.

Resetting QA Contact.

David can you attach a screen shot of what your proposal underlying the patch you posted would look like? Thanks!

Not a mail/news bug. Reassigning to ben.

I think dbaron's fix is the right idea but the BG colour is a little bright for me... how about using the same colour as is used for the hover in the taskbar (see attachment)

looking great! Let's do it!

fix checked in...

Vaguely menus-like bug; QA Assigning to Sarah for verification.

toolbars and all the widgets on 'em including menus belong to claudius... :-)

Reopening. The fix did not address the main issue of the bug, which is that *menus* within the personal toolbar are different from other things. Right now the buttons are great, but hover on menus doesn't do anything, and hover on items within the menus gives a different color from a hover on the buttons on the main personal toolbar.

Moving all UE/UI bugs to new component: User Interface: Design Feedback UE/UI component will be deleted.

this seems to work for me now... (02/22/00) I didn't touch anything, maybe someone else changed a style setting, or something was fixed. I'm marking worksforme - dbaron, if things still don't look right, please reopen again :)

Reopening. It *is* mostly fixed. However, the highlighting (:hover) color on the *items* within the menus is wrong (I would think it should be the same as the highlighting on the items on the toolbar itself). When fixing, do make sure that submenus of submenus (ad inf.) are handled correctly.

i'm pretty sure those colors are the ones that german wanted. he and hangas had me check those in a couple of days ago, and we commented on the difference.

hm.. although you're right, it might look nicer with subtler colours. I'll let german make this call, german, reassign back to me with a directive either way.

How come this is still M14? M14 is already out!

this should go to mcaff or anyone who is working on Linux. German can't fix this in the build. He is a UI designer :-) re-assigned to mcafee

The new personal toolbar design checked in recently fixes all these problems. All the comments on this bug relate to the two previous designs for the personal toolbar. Marking FIXED (and also All/All since it was never really Linux specific).

marking VERIFIED