406894 - Icons in the location bar and search bar need hover and depressed states (web feed)(star)(location bar)(bookmarks)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Gabriel Chadwick]

The star button should have a hover state IMO. This would improve consistency with the other buttons of the toolbars. It would also improve discoverability of the function. I would see kind of a glow around the star when hovering. Any toughs ? This could be wontfix too I don't know... Looked for a dupe did not find it...

Comment 1

[Ryan S. Scheel [:havvy]]

I haven't looked at the star function much, but maybe have the outline gray, and when on hover, make it black.

Comment 2

[Gabriel Chadwick]

Was kind of thinking something like the rss button.

Comment 3

[Matthew Middleton (:zzxc)]

The primary purpose of the star icon is to provide bookmark status, so any hover changes should be subtle.  (slight outline changes or subtle 'shimmer' effects, for example)

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

cc'ing faaborg for theme consistency: I can't remember if we're keeping hover states for buttons in textboxes anymore. I think we should, but until I get word, this is just wanted, not blocking.

Comment 5

[Gabriel Chadwick]

Any chance we get faaborg opinion ? 

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

On windows I think we should consider giving the RSS and Star icons the same visual treatment as what we are doing for the drop down marker (bug 397331).  We shouldn't do anything on OS X, and I'll let the tango team decide what makes the most sense on linux.

Comment 7

[Gabriel Chadwick]

Renominating for blocking following Faaborg opinion. Maybe this should be marked windows only ?

Comment 8

[Dão Gottwald [:dao]]

(In reply to comment #6)
> On windows I think we should consider giving the RSS and Star icons the same
> visual treatment as what we are doing for the drop down marker (bug 397331). 

For these icons I'd rather use opacity like for the search icon.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

This does not block the final release of Firefox 3.

Comment 11

[Alex Faaborg [:faaborg] (Firefox UX)]

>For these icons I'd rather use opacity like for the search icon.

The convention of buttons filling out the full height of the text field on hover is used very commonly throughout all of Vista (similar to what we are currently doing with the location bar drop down arrow, but using the same appearance as hovering on a dialog button).  I think we should consider the Vista approach on both XP and Vista since XP doesn't have a convention either way.

cc'ing beltzner to get his opinion.

Comment 12

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Examples of text field buttons in Vista

Here are some examples of buttons inside of text fields in Vista, taken from windows explorer and internet explorer.

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

Expanding the scope of this bug to cover all icon-based buttons inside of text fields.

Comment 14

[AndrewM]

What about the Go button? In bug 414389 comment 71 you said that the Go button should have a hover state, but it doesn't seem to be there yet.

Comment 15

[Alex Faaborg [:faaborg] (Firefox UX)]

I think the full set of buttons that need hover states in css (similar to what we are doing with the drop marker) are:

-Drop marker
-Star
-Web Feed
-Go
-Search

Comment 16

[Gabriel Chadwick]

Wouldn't the site button and search engine selector have a hover state too ?  

Comment 17

[Darren Kalck [:D-Kalck]]

(In reply to comment #16)
> Wouldn't the site button and search engine selector have a hover state too ?  
> 
I think they should have a hover state, especially the site button since there's not dropdown marker.

Comment 18

[Alex Faaborg [:faaborg] (Firefox UX)]

I am renominating this bug since not having any type of hover state makes the product feel unpolished, and this bug involves five controls in the main window.  If we don't want to do this with CSS we should use images, but I need to know soon if I should request them.

Comment 19

[Lim Chee Aun]

I 'm wondering if this needs additional -moz-appearance stuff since this can't be done with CSS? I manage to do the menulist-button on firefox3b5 though:

#urlbar > .autocomplete-history-dropmarker{
-moz-appearance: menulist-button !important;
margin: -1px -1px -1px 0 !important;
}

Comment 20

[Lim Chee Aun]

Attachment: Vista menulist button on Firefox 3b5

Comment 21

[AndrewM]

(In reply to comment #18)
> I am renominating this bug since not having any type of hover state makes the
> product feel unpolished, and this bug involves five controls in the main
> window.  If we don't want to do this with CSS we should use images, but I need
> to know soon if I should request them.

Also, it should be pointed out that the lack of a hover state for the web feed icon and Go button is a regression from Firefox 2, which had hover states for both of those icons. (The hover state for the web feed icon in particular looked fantastic :)

Comment 22

[AndrewM]

(In reply to comment #20)
> Created an attachment (id=313893) [details]
> Vista menulist button on Firefox 3b5

That looks really good, can I get that just by adding the code in comment 19 to userChrome.css? Or is it more complicated?

Also, I like how the drop-down arrow is the native Vista one (and the same one used for drop-down menus in web pages of course), rather than the smaller version as it is currently. Is there a way to use that for the 'Display Opened Tab List' arrow as well? (I think that would look also better if it were the bigger, Vista arrow.)

Comment 23

[Lim Chee Aun]

(In reply to comment #22)
> (In reply to comment #20)
> > Created an attachment (id=313893) [details] [details]
> > Vista menulist button on Firefox 3b5
> 
> That looks really good, can I get that just by adding the code in comment 19 to
> userChrome.css? Or is it more complicated?

Not complicated. Just that code.

> Also, I like how the drop-down arrow is the native Vista one (and the same one
> used for drop-down menus in web pages of course), rather than the smaller
> version as it is currently. Is there a way to use that for the 'Display Opened
> Tab List' arrow as well? (I think that would look also better if it were the
> bigger, Vista arrow.)

Nope, I don't think it'll work, unless more native UI widgets can be squeezed out from Vista. :)

Comment 24

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #19)
> #urlbar > .autocomplete-history-dropmarker{
> -moz-appearance: menulist-button !important;
> margin: -1px -1px -1px 0 !important;
> }

Can we get a separate bug on that filed with that patch nominated for review and approval? That looks a lot nicer for Vista than what we have now.

(In reply to comment #18)
> I am renominating this bug since not having any type of hover state makes the
> product feel unpolished, and this bug involves five controls in the main
> window.  If we don't want to do this with CSS we should use images, but I need
> to know soon if I should request them.

Not blocking, but we'd accept a safe, tested patch.

Comment 25

[Asa Dotzler [:asa]]

this is mostly fixed on Mac with the latest proto landing.

Comment 26

[Gabriel Chadwick]

Will Firefox 3 be released without this being fixed. This would be a major discoverabillity regression from Firefox 2. Starting to look like Firefox 3 is not being released but kicked out of the door.

Comment 27

[Alex Faaborg [:faaborg] (Firefox UX)]

>Will Firefox 3 be released without this being fixed. This would be a major
>discoverabillity regression from Firefox 2. Starting to look like Firefox 3 is
>not being released but kicked out of the door.

At a minimum we will do icon changes similar to Firefox 2.  We are generating alternate states of icons last so we don't have to regenerate them for every revision.  Given that we haven't produced a release candidate yet I don't think the "kicked out the door" analogy is fair.

Comment 28

[Gabriel Chadwick]

(In reply to comment #27)

> Given that we haven't produced a release candidate yet I don't think
> the "kicked out the door" analogy is fair.

Unasked for provocation from my part. Some tough desision are made in the shipping process and frustation can arise.

I understand the wait now. Thank you for your explanation and hard work. Firefox 3 is looking good !

Comment 29

[AndrewM]

(In reply to comment #24)
> (In reply to comment #19)
> > #urlbar > .autocomplete-history-dropmarker{
> > -moz-appearance: menulist-button !important;
> > margin: -1px -1px -1px 0 !important;
> > }
> 
> Can we get a separate bug on that filed with that patch nominated for review
> and approval? That looks a lot nicer for Vista than what we have now.

Filed bug 429310.

Comment 30

[Alex Faaborg [:faaborg] (Firefox UX)]

When the next drop of windows icons lands (bug 430759), alternate states for all of the text field icons will be included.  All of the files are in winstripe/browser:

bookmark-aero.png
bookmark.png

editBookmark-aero.png
editBookmark.png

feed-icons-aero.png
feed-icons.png

I believe go and search are both fine now, but it might be worth checking search since the previous icon had four states and the new one only has three (it can't be disabled).

I have also included all of the states of search close, although I don't believe this will be used in Firefox 3 since the search widget didn't make it.

Comment 31

[Alex Faaborg [:faaborg] (Firefox UX)]

bookmark-aero.png
bookmark.png

editBookmark-aero.png
editBookmark.png

These files have moved to places

Comment 32

[Alex Faaborg [:faaborg] (Firefox UX)]

feed-icons-16-aero.png
feed-icons-16.png

These files have moved to feeds

Comment 33

[Kai Liu]

Attachment: patch

Since nobody is taking this bug...

In addition to adding hover states, this patch also addresses the issues in bug 425939 and bug 430962 comment 10.

Comment 34

[Kai Liu]

Attachment: after screenshot

Comment 35

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 318028 [details] [diff] [review]
patch

This means we need to remove pageStarred.png and starPage.png, right?

+#feed-button:not([feeds]) {
+  visibility: collapse;

Why is this needed?

Comment 36

[Kai Liu]

(In reply to comment #35)
> (From update of attachment 318028 [details] [diff] [review])
> This means we need to remove pageStarred.png and starPage.png, right?
> 
AFAIK, nothing else refers to them, so I guess so.

> +#feed-button:not([feeds]) {
> +  visibility: collapse;
> 
> Why is this needed?
> 

Without that, all the spacing surrounding the feed button will remain.  With the collapse, the gap between the end of address bar text and the start of the address bar icons/buttons will remain the same whether or not the feed button is present, whereas without the collapse, that gap will be greater when the feed button is not present.

Comment 37

[Kai Liu]

(In reply to comment #36)
> (In reply to comment #35)
> > (From update of attachment 318028 [details] [diff] [review] [details])
> > This means we need to remove pageStarred.png and starPage.png, right?
> > 
> AFAIK, nothing else refers to them, so I guess so.
> 
So should I be removing the files from jar.mn with this patch?  Or is that something best left for the icon cleanup at the end when all the theme dust have settled?

Comment 38

[:Gavin Sharp [email: [e-mail]]]

Yeah, we should remove them now. You can also use cvsdo (from http://viper.haque.net/~timeless/redbean/ ) to fake cvs removing the files locally, so that when you |cvs diff| with -N the removals are shown in the diff.

Comment 39

[Kai Liu]

Attachment: patch & file removal

Same patch as above.  Delete the unused files.

Carrying over Gavin's review.

Comment 40

[Damon Sicore (:damons)]

Comment on attachment 318186 [details] [diff] [review]
patch & file removal

a1.9+=damons

Comment 41

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/themes/winstripe/browser/browser.css 	1.212
mozilla/browser/themes/winstripe/browser/jar.mn 	1.94
mozilla/browser/themes/winstripe/browser/places/pageStarred-aero.png delete	 
mozilla/browser/themes/winstripe/browser/places/pageStarred.png delete
mozilla/browser/themes/winstripe/browser/places/starPage-aero.png delete 
mozilla/browser/themes/winstripe/browser/places/starPage.png delete

Comment 42

[Gabriel Chadwick]

Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; fr; rv:1.9pre) Gecko/2008043007 Minefield/3.0pre ID:2008043007

Comment 43

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9) Gecko/2008051206 Firefox/3.0 ID:2008051206