581193 - button[type="menu-button"] looks like a dropdown, but acts like a button

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[Jesse Ruderman]

Steps:
1. Load http://htmlfive.appspot.com/static/whereami.html

It looks like clicking "Share Location ▾" will open a dropdown with options, but only the "▾" part acts like a dropdown.  This caused me to accidentally share my location with a site when I wanted to see what the options were.

Comment 1

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

Plan is to add a clear separator between the main action and the drop down for alternate actions.  Mockups of the final visual design are at the bottom of this post: http://blog.stephenhorlander.com/2010/07/01/geolocation-icons/

Comment 2

[Dão Gottwald [:dao]]

Attachment: patch

This doesn't attempt to fix bug 573599 but makes menu buttons usable again. Bug 573599 may end up in browser/ anyway, as it doesn't seem that we have the native widget support to implement the intended look robustly in toolkit.

Comment 3

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

Can you explain what this does? I don't see a difference on Mac...

Comment 4

[Dão Gottwald [:dao]]

It styles the inner button (rather than the outer one) like a button, in order to separate the inner button from the dropmarker.

Comment 5

[Dão Gottwald [:dao]]

e.g.:

.--------.
| Foo    | v
`--------´

instead of:

.-----------.
| Foo     v |
`-----------´

Comment 6

[Neil Deakin]

This doesn't look good on Mac, since the arrow is black and the geolocation prompt has a black background, so it isn't visible.

I think it would be better to just implement bug 573599 rather than move the arrow outside the button only to move it back again later.

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #6)
> This doesn't look good on Mac, since the arrow is black and the geolocation
> prompt has a black background, so it isn't visible.

This will be fixed in bug 577931, in a browser-specific way. Black isn't the standard background I care about here.

Comment 8

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

It doesn't really look good on Windows either (the dropdown is strangely disconnected from the button). Bug 577927 (and its dependents) covers the doorhanger UI specifically, and bug 573599 covers the general styling of menu-buttons, so it seems to me like this bug is redundant.

Comment 9

[Dão Gottwald [:dao]]

The widget is currently unusable. I'd like to have this fixed before even thinking about how it would look ideally. Also, bug 573599 is the follow-up to a doorhanger-motivated bug with doorhanger-specific mock-ups, so as I said, it might end up being fixed in browser-specific code (i.e. what's happening in bug 577931).

Comment 10

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

Comment 0 does not describe "unusable". It's far from ideal, but it's certainly functional. I think this is a duplicate of bug 573599, and we should just fix menu-buttons in general using an approach similar to the one in bug 577931.

Comment 11

[Dão Gottwald [:dao]]

Well, clicking it does something, but not what the user expected at all. "This caused me to accidentally share my location with a site when I wanted to see what the options were" is a fundamental failure. I don't think this qualifies as functional.

I don't know of a plan for getting bug 573599 done in a non-doorhanger-specific way.

Comment 12

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

What's "doorhanger specific" about the approach in bug 577931? We wouldn't want the darkness of the "hud" styling, but otherwise the button looks pretty good (though not native - is that what you're talking about?).

Comment 13

[Dão Gottwald [:dao]]

(In reply to comment #12)
> What's "doorhanger specific" about the approach in bug 577931? We wouldn't want
> the darkness of the "hud" styling, but otherwise the button looks pretty good
> (though not native - is that what you're talking about?).

The color as well as the affordance. So yes, it's not native. The base styling for button[type="menu-button"] shouldn't be drastically different from button or  button[type="menu"], though different enough to communicate how it works.

Comment 14

[Neil Deakin]

On Windows, the 'split button' type looks very similar to what we have now, but with a vertical line between the label and the arrow. Why can't we just do that? Mac and Linux do not provide native split buttons so there isn't anything to compare to there, but they could be done in a similar way. This doesn't seem like it needs to be that complicated.

The notification popup button could be styled in a more elaborate way on Mac, which is the only extra piece I think is needed.

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #14)
> On Windows, the 'split button' type looks very similar to what we have now, but
> with a vertical line between the label and the arrow. Why can't we just do
> that?

It would only superficially look right, the various button states would always affect the appearance of the whole button.

Comment 16

[imradyurrad]

Pardon the spam but can a different reviewer be assigned? There's been more than a month of inactivity and this is blocking.

Comment 17

[:Margaret Leibovic]

I have a patch in bug 577931 that makes the hud-style split buttons look right on OSX, and I'm working on patches for Windows 7 and Linux in bug 577928 and bug 577930. Right now these patches just fix the buttons in the doorhanger notifications, but eventually we could use them to style buttons elsewhere. 

Also, it doesn't look like the patch here styles the right side of the button when it is active, and that has been the main challenge in my patches, since I'm trying to make the buttons look like this: https://bug509642.bugzilla.mozilla.org/attachment.cgi?id=439672.

Comment 18

[Darren Kalck [:D-Kalck]]

Margaret, are you interested into fixing bug #589146

Comment 19

[Dão Gottwald [:dao]]

(In reply to comment #17)
> I have a patch in bug 577931 that makes the hud-style split buttons look right
> on OSX, and I'm working on patches for Windows 7 and Linux in bug 577928 and
> bug 577930. Right now these patches just fix the buttons in the doorhanger
> notifications, but eventually we could use them to style buttons elsewhere.

The Windows patch hard codes a lot, which is only acceptable for the default OS theme. As said before here, that patch also makes the button look different from other buttons, which is okay for the doorhanger popup but not for random other contexts.

The hard coded stuff in the Linux patch may not be acceptable at all, I've yet to take a closer look there.

Comment 20

[Neil Deakin]

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

Why are you asking me to review the same patch I've already denied?

Comment 21

[Dão Gottwald [:dao]]

(In reply to comment #20)
> Comment on attachment 459818 [details] [diff] [review]
> patch
> 
> Why are you asking me to review the same patch I've already denied?

You didn't comment. I thought it was based on comment 17, which I responded to.

Comment 22

[:Margaret Leibovic]

Where are these split buttons currently used, other than in the doorhanger notifications? If they're not used elsewhere, this might not need to block. (This bug was set to blocking before I wrote the fix for the doorhanger buttons.)

Comment 23

[Darren Kalck [:D-Kalck]]

In the firefox menu, I think. So it should really block.

Comment 24

[Dão Gottwald [:dao]]

Ugh, please don't mess with the blocking flag just because you had a question.

Add-ons can use menu buttons, and the popup notification styling is limited to the default themes on Windows (Luna/Aero) and non-existent on Linux.

This has nothing to do with the Firefox menu.

Comment 25

[Patrick Walton (:pcwalton)]

(In reply to comment #22)
> Where are these split buttons currently used, other than in the doorhanger
> notifications? If they're not used elsewhere, this might not need to block.
> (This bug was set to blocking before I wrote the fix for the doorhanger
> buttons.)

They're used in the Web Console, but they're manually styled there.

Comment 26

[Dão Gottwald [:dao]]

The web console uses <toolbarbutton type="menu-button"/> rather than <button type="menu-button"/>.

Comment 27

[Patrick Walton (:pcwalton)]

(In reply to comment #26)
> The web console uses <toolbarbutton type="menu-button"/> rather than <button
> type="menu-button"/>.

Ah, right. Carry on.

Comment 28

[Jim Porter (:squib)]

(In reply to comment #22)
> Where are these split buttons currently used, other than in the doorhanger
> notifications? If they're not used elsewhere, this might not need to block.
> (This bug was set to blocking before I wrote the fix for the doorhanger
> buttons.)

Thunderbird uses them in a few places, notably the message toolbar, where - although they're toolbarbuttons - they're styled as regular buttons.

Comment 29

[Dão Gottwald [:dao]]

fixed by backing out bug 509642