405791 - Change mouse pointer for buttons inside of text entries

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Michael Monreal [:micmon]]

The Go, Star and Search "buttons" recently moved inside of the url and search entries. It would be cool if the mouse pointer would change when hovering on top of such a button.

GTK has similar widgets with integrated "buttons" (see evolution for example) and also changes the pointer to a hand when hovering those. GTK uses this method instead of having different states of the actual image (pressed/active/hover/normal). Mozilla already changes the pointer similarly for www links.

Comment 1

[Michael Monreal [:micmon]]

Implementing this is very easy btw, as CSS allows doing it. Using the hand pointer for Go, Feed, Star, Search (all main toolbar) and Clear-Search (in Places Organizer):

#go-button, #feed-button, #star-button, .search-go-button,
.textbox-input-closebutton {
  cursor: pointer !important;
}

Comment 2

[Michael Monreal [:micmon]]

Also, the mouse pointer when draging the favicon looks bad, this would fix it:

#page-proxy-button {
  cursor: move !important;
}

Comment 3

[Michael Monreal [:micmon]]

Ping. Some comment would be great, else we will probably need to look into designing separate states for the Go, Star and Search icons.

Comment 4

[Julien "_FrnchFrgg_" RIVAUD]

Michael: Do you have a custom build or not ? You probably should (and I can help you to), if you want to be able to write patches. Without patches your bugs won't go very far, and patches to the CSS are most of the time as easy to write as userChrome.css...

I'll write a patch for you this time. Note: There is a #urlbar-icons. Can we use it to change the pointer (or change the pointer on #urlbar-icons > *), or are there some icons you don't want to have this behaviour ?

Comment 5

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Change cursor to "pointer" (hand) when hovering url bar and search bar icons

That could be a patch. Tell me what you think about it.

Comment 6

[Michael Monreal [:micmon]]

Thanks again _FrnchFrgg_. This looks about right. The question is, do other people like the idea? I think it really helps discovering the icons as pressable (mostly after we remove all the hover/pressed/etc states)

Comment 7

[Michael Ventnor]

Why not set the hover icon on each individual icon? That way any spacing between the icons won't get the feedback and we can put in non-clickable icons too.

Comment 8

[Julien "_FrnchFrgg_" RIVAUD]

Ventnor: The pb with this approach is that we'll need to change the rule everytime we add a new clickable icon. But indeed, I thought about that when I said:

>Can we use it to change the pointer (or change the pointer on #urlbar-icons > *),
>or are there some icons you don't want to have this behaviour ?

I can post a different patch if it is needed

Comment 9

[Michael Ventnor]

Yeah, that rule sounds better.

Comment 10

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Change cursor to pointer (hand) when hovering url bar and search bar icons [v2]

Comment 11

[Julien "_FrnchFrgg_" RIVAUD]

Comment on attachment 299623 [details] [diff] [review]
Change cursor to pointer (hand) when hovering url bar and search bar icons [v2]

This time with the rule

#urlbar > * { cursor: pointer }

It will make the cursor change only on the icons and not between them, and makes it easier to override for particular icons if needed.

Comment 12

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Comment on attachment 299623 [details] [diff] [review]
Change cursor to pointer (hand) when hovering url bar and search bar icons [v2]

>diff --git a/browser/themes/gnomestripe/browser/browser.css b/browser/themes/gnomestripe/browser/browser.css
>+.search-go-button {
This should be added to the existing rule in searchbar.css.

>+    cursor: pointer;
>+}
Nit: indent with two spaces.


>+#urlbar-icons > * {
Because of the way rules are matched, this means you're going to be checking if every element within browser.css' reach is a child of #urlbar-icons. You should either list the icons out or create a new class to assign to them.

Comment 13

[Julien "_FrnchFrgg_" RIVAUD]

Which way do you prefer ? Listing the icons out carries the risk that when an icon will be added, the rule won't be updated (and this isn't a very visible bug), but adding a new class... adds a new class.

Comment 14

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Change cursor to pointer (hand) when hovering url bar and search bar icons [v3]

Comment 15

[Asaf Romano (gone)]

Comment on attachment 300664 [details] [diff] [review]
Change cursor to pointer (hand) when hovering url bar and search bar icons [v3]

mpa=mano

Comment 16

[Reed Loden [:reed]]

Checking in browser/base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.430; previous revision: 1.429
done
Checking in browser/themes/gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.175; previous revision: 1.174
done
Checking in browser/themes/gnomestripe/browser/searchbar.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/searchbar.css,v  <--  searchbar.css
new revision: 1.25; previous revision: 1.24
done