1253419 - Support two-part browserAction buttons with a primary action and a secondary menu

Status: RESOLVED WONTFIX

(WebExtensions :: Frontend, enhancement)

Comment 1

[Caitlin Neiman [:caitmuenster]]

To be discussed during Nov. 15, 2016 web extensions triage meeting.

Comment 2

[Andy McKay]

Kris, could describe this feature in a bit more detail please?

Comment 3

[Kris Maglione [:kmag]]

Basically, we just need to create a menu-button type toolbar button, and then use something like contextMenus API to allow the extension to add menu items to it.

Comment 4

[Martin Giger [:freaktechnik]]

This would be a secondary menu button (a little arrow pointing down) and the main action of the page action would still be something different?

Comment 5

[Kris Maglione [:kmag]]

Yes.

Comment 6

[brunoais]

Are you open for someone to make a proposal for this through a legacy addon exposing an API for a web extensions addon to use?

Comment 7

[Andy McKay]

(In reply to brunoais from comment #6)
> Are you open for someone to make a proposal for this through a legacy addon
> exposing an API for a web extensions addon to use?

Could you expand on that please, I'm not sure I follow.

Comment 8

[brunoais]

I write a legacy extension with the ideas of these docs:
https://webextensions-experiments.readthedocs.io/en/latest/how.html
and the same ideology as:
https://github.com/web-ext-experiments/logins

Then I do an example Web Extension that exemplifies of the new API in action just like logins example already does.

Comment 9

[Andy McKay]

Ah right, that sounds great.

Comment 10

[brunoais]

I won't be able to start on this in this month but I can start working on this in December. Anyone who wants to do it instead of me, feel free to do it and warning here to avoid both doing the same thing.

Comment 11

[Andy McKay]

Attachment: Example from adblock plus

Comment 12

[brunoais]

Attachment: Greasemonkey example

I prefer how they did with greasemonkey.
The vertical drawing in the middle making it clear there are two buttons seem better IMO.
Oh! By the way,
does anyone think we can allow any very small image or character instead of forcing the use of an inverted triangle?

Comment 14

[flying sheep]

> I prefer how they did with greasemonkey. The vertical drawing in the middle making it clear there are two buttons seem better IMO.

That’s the default appearance for <button type="menu-button">. And I also like it better. In Qt, a button with ABP’s appearance would behave like a BrowserAction with panel, i.e. it’s a (non-segmented) button that signifies that it triggers no immediate action but opens a panel.

> does anyone think we can allow any very small image or character instead of forcing the use of an inverted triangle?

sure! actually the segmented bookmark button is implemented like this. its right half is the arrow part of a menu-button, with overriden icon.

Comment 15

[flying sheep]

by the way: bug 1320876 isn’t actually a duplicate of this. there’s also real combined buttons, which that one is about.

those combined buttons could also be used to get a segmented button with multiple icons.

Comment 16

[Sergio]

(In reply to Kris Maglione [:kmag] from comment #3)
> Basically, we just need to create a menu-button type toolbar button, and
> then use something like contextMenus API to allow the extension to add menu
> items to it.

Or not just menu buttons but panels too, I've an extensions that uses the drop marker to open a panel and the main button to do the main addon function.

Comment 17

[Andreas Wagner [:TheOne] [use NI]]

Personas Plus would need this (in case we want to keep the same UI).

Comment 18

[Michael Buckley]

Attachment: An implimentation of how this could be done.

Here is a patch that I put together, not the most elegant solution, but it works well.

It defines a new entry for the manifest "dropmarker_popup" works kind of like "default_popup" except it only gets shown when the user clicks on the dropmarker.

I just released as I write this, I forgot to write the getter and setter methods for it. But that should just be a copy and paste job. I will do it another day, I have spent more time then I have already on this for now.  I am interested in feedback on what people think of this approach? If it is good, I will add a completed patch with the missing methods.

With adding icons.  Should it be just add something like "dropmarker_icons" that works the same as "default_icons" or should it simply be a single image?

Comment 19

[flying sheep]

If the button is a menu-button, then the click-event’s originalTarget is another DOM-object with the class “toolbarbutton-menubutton-button”. Checking this is more robust than your click position check.

Note that this element is only available if type==="menu-button"

Comment 20

[Michael Buckley]

flying sheep, I did try that at first as well (well not exactly, but a variation of it). The problem is I am looking for the dropmaker, not that button.  You can't just assume that every click that is not on the button is on the dropmaker.  There is for example a space of about 2px (a guess) wide on the left side of the button. As I said in the last post, it is not elegant, but it works.

The first thing I was looking for was to act directly on the dropmaker. But the originalTarget is never set to the dropmaker. Also you don't appear to be able to attach events directly to the dropmaker, they never fire.

Comment 21

[flying sheep]

I don't think that problem should be fixed here.

Hovering those 2 pixels will also give the visual feedback that the icon part of the button isn't hovered, which also needs to be fixed.

We should directly fix the CSS of menu buttons until the hover style and therefore behavior matches what we want.

We could use invisible borders or even pointer-events:none for that.

Comment 22

[Michael Buckley]

So what do you think add something like this to the browser.css (somewhere after the 'toolbar .toolbarbutton-1' which is around line 724)?

toolbar .toolbarbutton-1[type="menu-button"] {
    padding:0;
}

The 2px should be redundant anyway since the toolbarbutton inside it has the padding.  I could put it in another patch, and post it as another bug that is blocking this one (if that is the correct way to proceed).

The test that I would make is not for the class then but rather if event.originalTarget.getAttribute('anonid') == 'button'.  I think would make more sense.

Let me know, and when I have time I will try and get the patches together, along with finishing the implementation (the missing methods, plus I will try adding an icon).

What version of Firefox could I realistically hope to get this into? I kind of want it for a few of my add-ons.

Comment 23

[flying sheep]

I wouldn’t say it’s blocking. It’s more like a minor UX problem that surfaces here, but can be fixed independently.

If we still have a clickable rim after this CSS fix, we could also e.g. go with giving the icon a transparent (but clickable) border and corresponding negative margin.

I also want it for my addons. I really think they should provide a way to fast-track Addon API patches, or there’ll be a long period in which some addons are broken.

Comment 24

[Hector Zhao [:hectorz]]

FYI, just noticed the styles for `toolbarbutton[type="menu-button"]` was removed in bug 1387253, because "WebExtensions can't add such buttons".

Comment 25

[flying sheep]

this just means the reverse patch has to be added to this one. How are they coming along, btw?

Comment 26

[Dão Gottwald [:dao]]

(In reply to flying sheep from comment #25)
> this just means the reverse patch has to be added to this one. How are they
> coming along, btw?

It's a lot of styling that was removed and I don't think we're ready to commit to maintaining this again.

Comment 27

[flying sheep]

I can only interpret this the following way, which can’t be true for a contribution friendly project:

> The order of patches matters. Contributing to firefox is a
> race where the faster patcher wins and shifts the goal posts.
> Patches will be rejected if they’re slightly too late.

I hope you’re not saying this and there’s a way to get this patch (+necessary styling) in.

Comment 28

[Karoly Pados]

+1 in support for this ticket. As an add-on developer, I also want a way to show a menu added using the contextMenu API on left-click on toolbar (or address bar) button.

Comment 29

[Dão Gottwald [:dao]]

(In reply to flying sheep from comment #27)
> I hope you’re not saying this and there’s a way to get this patch
> (+necessary styling) in.

I don't think we'll add the styling back. However I expect this could be implemented as two <toolbarbutton>s within a <toolbaritem> rather than <toolbarbutton type="menu-button">.

Comment 30

[flying sheep]

Sure, whatever works! The two-part bookmarks button works that way.

And we could add menu-buttons together with multipart-buttons in that style, further enhancing addon authoring possibilities.

Comment 31

[Michael Buckley]

Sorry for no progress on this, been really busy.  Going on holidays next week, so hopefully have some time to play around with this again.

Implementing it as two toolbarbuttons within a toolbaritem would be reinventing the wheel, and also means you would have to add even more styling to make it work.  It would also greatly increase the complexity of the code.  In fact I would suspect a total rewrite of how ext-browserAction.js works might be called for.  

In short avoiding <toolbarbutton type="menu-button"> simply because you don't want to bring some style rules back would be very counter productive.

Comment 32

[Dão Gottwald [:dao]]

Bug 1434860 completely removed support for <toolbarbutton type="menu-button"/>.

Comment 33

[David Durst [:ddurst]]

Clearing priority and flags for re-triage.

Comment 34

[Rob Wu [:robwu]]

The built-in UI functionality has been removed in bug 1434860, and there has been very little interest in this bug in the past two years.

Extensions can define context menu items on the extension button, or use custom HTML (e.g. a tabbed UI) in the extension popup to provide access to functionality.

There is also a different, approved feature request to support multiple actions depending on mouse button / keyboard modifiers in bug 1405031.

Comment 35

[Sergio]

there has been very little interest in this bug in the past two years

Please tell me how can we show interest in a bug without posting on bugzilla, cause if we post here the post are hidden as off topic, advocacy or similar cause this is not the place to do that.

I need to know ASAP so i can show interest in bugs that are still open so they don't close them forever as this one so one day I'll be able to port to web-extensions without losing half the features.

Extensions can define context menu items on the extension button, or use custom HTML (e.g. a tabbed UI) in the extension popup to provide access to functionality

Of course we can use the extension UI to achieve the same just with one additional click... lately I've seen this approach on a few UI designs moving everything one, or even more, clicks away than before just so we can have less buttons showing at once.

For my it seems like trading "better looking" for commodity/speed. ¯\(ツ)/¯

Comment 36

[Rob Wu [:robwu]]

(In reply to Sergio from comment #35)

there has been very little interest in this bug in the past two years

Please tell me how can we show interest in a bug without posting on bugzilla, cause if we post here the post are hidden as off topic, advocacy or similar cause this is not the place to do that.

I need to know ASAP so i can show interest in bugs that are still open so they don't close them forever as this one so one day I'll be able to port to web-extensions without losing half the features.

This bug was closed because it seemed unlikely that we would ever implement this (as the UI did not exist any more, and the supporting internals were gone). The fact that there has been little activity on this bug was not the deciding factor, but it did help with confirming that the bug could be closed.

In general, if a bug is closed while it should not have been, then you could post a comment and respectfully explain why you believe that a bug should not have been closed. If the comment is convincing, a bug may be reopened. The number of people that work on this is not infinite, so we cannot implement every requested feature and need to prioritize.