Closed Bug 1253419 Opened 4 years ago Closed 4 months ago

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

Categories

(WebExtensions :: Frontend, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: kmag, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [browserAction][design-decision-needed])

Attachments

(3 files)

No description provided.
Whiteboard: [browserAction] → [browserAction][berlin]
Priority: -- → P3
Whiteboard: [browserAction][berlin] → [browserAction][berlin]triaged
Whiteboard: [browserAction][berlin]triaged → [browserAction][triaged][design-decision-needed]
Component: WebExtensions: Untriaged → WebExtensions: Frontend
To be discussed during Nov. 15, 2016 web extensions triage meeting.
Kris, could describe this feature in a bit more detail please?
Flags: needinfo?(kmaglione+bmo)
Whiteboard: [browserAction][triaged][design-decision-needed] → [browserAction][triaged][design-decision-approved]
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.
Flags: needinfo?(kmaglione+bmo)
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?
Yes.
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?
(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.
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.
Flags: needinfo?(amckay)
Ah right, that sounds great.
Flags: needinfo?(amckay)
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.
Attached image 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?
Assignee: nobody → kmaglione+bmo
Duplicate of this bug: 1320876
Assignee: kmaglione+bmo → tomica
> 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.
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.
webextensions: --- → ?
Summary: Support menu-button buttons in the pageAction API. → Support menu-button buttons in the browserAction API.
Assignee: tomica → kmaglione+bmo
Blocks: 1311472
(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.
webextensions: ? → ---
Personas Plus would need this (in case we want to keep the same UI).
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?
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"
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.
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.
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.
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.
FYI, just noticed the styles for `toolbarbutton[type="menu-button"]` was removed in bug 1387253, because "WebExtensions can't add such buttons".
See Also: → 1387253
this just means the reverse patch has to be added to this one. How are they coming along, btw?
(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.
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.
+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.
(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">.
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.
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.
Bug 1434860 completely removed support for <toolbarbutton type="menu-button"/>.
Summary: Support menu-button buttons in the browserAction API. → Support two-part browserAction buttons with a primary action and a secondary menu
Product: Toolkit → WebExtensions
Clearing priority and flags for re-triage.
Assignee: kmaglione+bmo → nobody
Priority: P3 → --
Whiteboard: [browserAction][triaged][design-decision-approved] → [browserAction][design-decision-needed]

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.

Status: NEW → RESOLVED
Type: defect → enhancement
Closed: 4 months ago
Resolution: --- → WONTFIX

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. ¯\(ツ)

(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.

You need to log in before you can comment on or make changes to this bug.