352681 - In text-only mode for toolbar, no hover effect for URL bar 'Go' and searchbar 'Search'

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20060914 BonEcho/2.0 ID:2006091404

1. New profile, start firefox
2. Right click on toolbar, choose Customize
3. Set to show 'text' only for toolbar
4. Click 'Done' to apply changes

Expected:
Go button at end of URL bar, and Search button at end of search bar should indicate that they are clickable when hovered over in the same way other clickable buttons are (bevel effect)

Actual:
No effect is applied to buttons, not clear that they are clickable.

Comment 1

[Marek Stępień [:marcoos, inactive]]

Also happens on Linux.

HW/OS -> All.

Comment 2

[Simon Bünzli]

This will probably be fixed by or together with bug 352682.

Comment 3

[Peter Kasting]

Actually, this should probably be fixed by the patch for bug 347754.

Comment 4

[Peter Kasting]

(In reply to comment #3)
> Actually, this should probably be fixed by the patch for bug 347754.

And by that I mean Pam's complete solution, not the band-aid patch I just posted there.

Comment 5

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

I don't think this is a blocker, but we'd take a patch if someone can put one together quickly.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: fixes problem

The solution to the problem is just to make the rules for a non-native toolbarbutton be specific to non-text icons mode.  So when we *are* in text icons mode, the toolbarbutton retains its default native style, and none of the button images get applied.

Comment 7

[Peter Kasting]

Clever, and should be safe.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: a better fix

The previous patch caused the location bar and search field to stretch vertically in text mode, since the text buttons are taller than those textboxes.  To fix this, we center align the buttons and the textboxes in text mode.  But we have to make sure that other elements (f.e. the search engine dropmarker) continue to stretch vertically, even in text mode, so we wrap all the elements except for the buttons in a stretchy box.

Note that the big chunks of apparent changes are merely indenting changes.  The only change I'm actually making to browser.xul and search.xml is to wrap those chunks in new hboxes.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: both better and simpler

I realized that we don't need to add the hbox in browser.xul, since the dropmarker (and everything else, for that matter) is inside the textbox, which is stretchy.  We still need the hbox in search.xml, though, since otherwise the search engine dropdown wouldn't stretch to the height of the search field.

I also moved the two center alignment CSS rules to more appropriate places within browser.css and searchbar.css.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 238600 [details] [diff] [review]
patch v3: both better and simpler

a=mconnor on behalf of drivers for 1.8 branch checkin

Comment 11

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20060916 BonEcho/2.0 ID:2006091603

This appears to have fixed the hover effect for the 'go' button in text mode, but not the 'search' button. In fact, it appears that in icon mode, now when you hover over the search bar icon (the magnifying glass) its background changes, but the icon itself doesn't.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Attachment: followup patch that fixes search button icon when hovering

> This appears to have fixed the hover effect for the 'go' button in text mode,
> but not the 'search' button. In fact, it appears that in icon mode, now when
> you hover over the search bar icon (the magnifying glass) its background
> changes, but the icon itself doesn't.

I see the problem when hovering over the search button in icon mode.  Here's the trivial fix for that.  I don't, however, see how the search button isn't fixed in text mode.  That seems to work for me.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

> I see the problem when hovering over the search button in icon mode.  Here's
> the trivial fix for that.  I don't, however, see how the search button isn't
> fixed in text mode.  That seems to work for me.

I rebuilt and tested again on Windows, and I still can't reproduce the problem with hovering over the Search button in text mode.  For me the Search button gains a beveled appearance just like the Go button in that mode.

I can, however, reproduce the problem with hovering over the Search button in icon mode.  The problem is that the CSS rule which determines which icon to display when the button isn't being hovered over has a higher precedence than the rule which determines which icon to display when it is being hovered over.  The patch just makes it so the hover rule takes precedence during hover.

I tested the patch on both Linux and Windows, and it fixes the problem in both places.

Note: I'll be out of town from this evening until tomorrow evening.  If the followup patch gets review and approval to land for rc1, and if it should go in before tomorrow evening, then someone will need to check it in for me.

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 238836 [details] [diff] [review]
followup patch that fixes search button icon when hovering

r+a=me, thanks!  (If you're around please land by 6 PM PDT, or I'll get it for you when I do a final pass)

Comment 15

[Myk Melez [:myk] [@mykmelez]]

Followup patch checked into the branch.  If you still don't see the hover effect on Windows, please reopen the bug, or better yet, file a new bug and mark it as a regression from this one.

Comment 16

[Steve England [:stevee]]

I have filed bug 353149 on the Search button in text-only mode not having a hover effect.