1448518 - Support registration of items in bookmarks folders in toolbar with a custom (non-navigation) action (e.g. with menus.create)

Status: NEW

(WebExtensions :: General, enhancement, P5)

Description

[Nye Liu]

What problem would this feature solve?
======================================
Trying to override (or extend) Open all in Tabs functionality



Who has this problem?
=====================
All contributors to MDN

How do you know that the users identified above have this problem?
==================================================================
Many pre-quantum extensions relied on this. None of them work now.

How are the users identified above solving this problem now?
============================================================
There is no solution. Currently the "bookmark" context only allows adding to the RIGHT click bookmark context menu.

Do you have any suggestions for solving the problem? Please explain in detail.
==============================================================================
Add a contextMenus.create() context to allow adding items to the LEFT click menu of bookmark folders.

Is there anything else we should know?
======================================

Comment 1

[Nye Liu]

Sample addon that uses "bookmarks" context: note that it can only add items to the right click menu

https://addons.mozilla.org/en-US/firefox/addon/replace-all-in-tabs/

Comment 2

[Andrew Swan [:aswan]]

I don't understand what you're requesting, we don't show context menus on a left-click

Comment 3

[Nye Liu]

I understand that; there seems to be no way to add (or override) anything on left click bookmarks. "Open all in Tabs" is technically a "context" type item, but is in LEFT click.

I would like to override "Open all in Tabs" behavior, or add a new items that does something different; in this case, do not create new tabs, but load the bookmarks into existing tabs.

There used to be a (builtin) option to adjust that behavior of "open all in tabs" but it is gone. I would like to add it back.

Comment 4

[Nye Liu]

The option was "browser.tabs.LoadBookmarksInTabs"

The very old behavior was to ALWAYS replace in tabs, then they added that option. All was good. Then they completely made "new tabs" the default and (sadly) *REMOVED* the about:config option.

Comment 5

[Nye Liu]

Also, to complicate matters further, there is no way to really implement real bookmark launching because things like about:config etc. cannot be opened using tab.create() or .update()

Comment 6

[Mike Conca [:mconca]]

Attachment: Capture.PNG

Attached is an image showing the menu item being discussed.

Comment 7

[Caitlin Neiman [:caitmuenster]]

Hi Nye, this has been added to the agenda for the WebExtensions APIs triage on May 8. Would you be able to join us? 

Here’s a quick overview of what to expect at the triage: 

* We normally spend 5 minutes per bug
* The more information in the bug, the better
* The goal of the triage is to give a general thumbs up or thumbs down on a proposal; we won't be going deep into implementation details

Relevant Links: 

* Wiki for the meeting: https://wiki.mozilla.org/WebExtensions/Triage#Next_Meeting
* Meeting agenda: https://docs.google.com/document/d/1TRkZ2u3GQXwlHpfP4-P_SXcwIwvlQz5Sm-C8I89Ya_o/edit#
* Vision doc for WebExtensions: https://wiki.mozilla.org/WebExtensions/Vision

Comment 8

[Tim Nguyen :ntim]

I think it's reasonable to make contexts: ["bookmarks"] apply to this menu as well.

Comment 9

[Tim Nguyen :ntim]

Alternatively, this can be a different context type, like "bookmarks-folder".

Comment 10

[Nye Liu]

Catlin: I can be at the meeting, depending on what time it is, I have a few other things on my calendar that day.

Tim: A different context type would be preferable because otherwise any new item will appear in both.

As an aside, if this feature is added, it would also be good to make sure that any addons using partially supported context type (depending on browser version) can be made to be compatible with both old and new browsers without making two versions of the addon.

Thanks!

Comment 11

[Nye Liu]

Also note that since we are talking about LEFT click and not RIGHT click, the "context" in "contextMenus" (which generally means RIGHT click) is a misnomer, as opposed to the "context" option of create() itself.

I do not if there is some sort of nomenclature consensus that would help clear up that confusion.

Comment 12

[Shane Caraveo (:mixedpuppy)]

(In reply to Nye Liu from comment #4)
> The option was "browser.tabs.LoadBookmarksInTabs"
> 
> The very old behavior was to ALWAYS replace in tabs, then they added that
> option. All was good. Then they completely made "new tabs" the default and
> (sadly) *REMOVED* the about:config option.

I don't understand this.   loadBookmarksInTabs is still in prefs and about:config.

(In reply to Nye Liu from comment #11)
> Also note that since we are talking about LEFT click and not RIGHT click,
> the "context" in "contextMenus" (which generally means RIGHT click) is a
> misnomer, as opposed to the "context" option of create() itself.

I don't see anywhere that we make a distinction on right vs. left context clicks, we simply have context menus that are initiated by mouse button 2, which is probably dependent on OS settings (e.g. does RTL change the meaning of mouse button numbers).  Adding support for different context menus dependent on which mouse button is used would be subject to UX approval.  I don't see a need for this.

See https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button

(In reply to Tim Nguyen :ntim (busy until May 25th) from comment #9)
> Alternatively, this can be a different context type, like "bookmarks-folder".

This seems like the most straightforward approach, but that doesn't "override" existing context menus per comment 0.

I think "bookmarks" should apply to both, "bookmarks-folder" applies only to folders.

I'm not really clear what this bug is about.  Subject is clear, OP keeps talking about right click context.

Comment 13

[Shane Caraveo (:mixedpuppy)]

Ahh, I understand now.  

This is a confusion between the context menu on a bookmark folder and the bookmark folder action which produces a menu (not a context menu).  The request here it to modify the (non-context) menu that clicking on the bookmark folder produces.

Comment 14

[Nye Liu]

Sorry about the digression, but it is apropo to motivation:

(In reply to Shane Caraveo (:mixedpuppy) from comment #12)
> (In reply to Nye Liu from comment #4)
> > The option was "browser.tabs.LoadBookmarksInTabs"
> > 
> > The very old behavior was to ALWAYS replace in tabs, then they added that
> > option. All was good. Then they completely made "new tabs" the default and
> > (sadly) *REMOVED* the about:config option.
> 
> I don't understand this.   loadBookmarksInTabs is still in prefs and
> about:config.

It no longer does anything useful. It used to alter the "Open all in tabs" behavior between "Open bookmarks in *new* tabs" and "Load bookmarks in *existing* tabs".

> The request
> here it to modify the (non-context) menu that clicking on the bookmark folder
> produces.

Yes!

Comment 15

[Shane Caraveo (:mixedpuppy)]

(In reply to Nye Liu from comment #14)
> Sorry about the digression, but it is apropo to motivation:
> 
> (In reply to Shane Caraveo (:mixedpuppy) from comment #12)
> > (In reply to Nye Liu from comment #4)
> > > The option was "browser.tabs.LoadBookmarksInTabs"
> > > 
> > > The very old behavior was to ALWAYS replace in tabs, then they added that
> > > option. All was good. Then they completely made "new tabs" the default and
> > > (sadly) *REMOVED* the about:config option.
> > 
> > I don't understand this.   loadBookmarksInTabs is still in prefs and
> > about:config.
> 
> It no longer does anything useful. It used to alter the "Open all in tabs"
> behavior between "Open bookmarks in *new* tabs" and "Load bookmarks in
> *existing* tabs".

That doesn't look like the case.

https://searchfox.org/mozilla-central/rev/b28b94dc81d60c6d9164315adbd4a5073526d372/browser/components/places/PlacesUIUtils.jsm#710

Specifically, if that pref is true it changes from "current" to "tab" which should cause new tabs to be created.  if this not working, open a new bug specifically for that.  A regression range would be nice also (see mozregression).

Comment 16

[Nye Liu]

(In reply to Shane Caraveo (:mixedpuppy) from comment #15)
> > It no longer does anything useful. It used to alter the "Open all in tabs"
> > behavior between "Open bookmarks in *new* tabs" and "Load bookmarks in
> > *existing* tabs".
> 
> That doesn't look like the case.

AFAIK it only affects loading single bookmarks, not the "Load all in tabs" item.

Comment 17

[Shane Caraveo (:mixedpuppy)]

(In reply to Nye Liu from comment #16)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #15)
> > > It no longer does anything useful. It used to alter the "Open all in tabs"
> > > behavior between "Open bookmarks in *new* tabs" and "Load bookmarks in
> > > *existing* tabs".
> > 
> > That doesn't look like the case.
> 
> AFAIK it only affects loading single bookmarks, not the "Load all in tabs"
> item.

If it is a regression (and everything you said previously indicates that), then create a new bug.

Comment 18

[Nye Liu]

It has been discussed to death, and unfortunately there is no resolution in sight, hence my addon :)

https://bugzilla.mozilla.org/show_bug.cgi?id=411029
https://bugzilla.mozilla.org/show_bug.cgi?id=440093
https://bugzilla.mozilla.org/show_bug.cgi?id=683146

Comment 19

[Nye Liu]

I should add, this behavior of loadBookmarksInTabs seems to be intentional, from what I gather from the threads (again, apologies for the digression).

See also bug 658774

Comment 20

[Nye Liu]

In particular:

Bug 409122
Bug 395024

Comment 21

[Nye Liu]

Looks like I was wrong, it was "browser.tabs.loadFolderAndReplace" that was removed, and not "browser.tabs.LoadBookmarksInTabs"

Comment 22

[Shane Caraveo (:mixedpuppy)]

Conceptually we're good with some additional menu contexts (context in API terms[1]).  We have a number of design issues we have to go over in regards to the menu api so we'll add this as part of that discussion.  So while we're approving it at this level, it will still get considered/resolved as a part of the larger design conversation we're having.

[1]https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/menus/ContextType

Comment 23

[Shane Caraveo (:mixedpuppy)]

Nye, if there is an issue with the pref we've been discussion, please split that out from this bug which is about the menu system and not the behavior of that pref.

Comment 24

[Nye Liu]

Shane, it looks like that horse left the barn years ago, and I don't want to fight over an ancient bug that is considered resolved by the design team. As long as web extensions allow developers to have the bookmark folder menu behave like they want, I don't see a reason to rock the boat and add cruft to mozilla itself.

If, on the other hand, you think the odds are good that I can convince everyone to resurrect LoadFolderAndReplace, let me know and I will reopen Bug 395024 or create a new bug to dispute it.

Again, apologies to all for the digression, I will let it go here.

Comment 25

[kernp25]

Is this really needed?
Because a user can also right-click on the folder and then choose the extension.
The extension then gets the links from the folder using the bookmarks api.
Does this menu only show for folders, that are in the toolbar?

Comment 26

[Nye Liu]

From the bug description:

How are the users identified above solving this problem now?

There is no solution. Currently the "bookmark" context only allows adding to the RIGHT click bookmark context menu.

Do you have any suggestions for solving the problem? Please explain in detail.

Add a contextMenus.create() context to allow adding items to the LEFT click menu of bookmark folders.

Comment 27

[Rob Wu [:robwu]]

(In reply to kernp25 from comment #25)

Is this really needed?
Because a user can also right-click on the folder and then choose the extension.
The extension then gets the links from the folder using the bookmarks api.
Does this menu only show for folders, that are in the toolbar?

As comment 26 notes, the API currently only offers a way to show a context menu on all bookmarks. Not on a subset, e.g. bookmark folders.

But the requested functionality can already be implemented, without new APIs:

1. Register a menu item for the "bookmark" context as usual.
2. Register a menus.onShown listener, and if the menu item from step 1 is matched, check its bookmarkId (e.g. with the bookmarks API) and show/hide the menu item if desired (using the "visible" key).

For documentation, see:

• https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/onShown
• https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/update
• https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/menus/refresh

Nye, would this work for you?

Comment 28

[Nye Liu]

Apologies, but I don't see how I can create an additional left click menu item in a bookmarks folder this way, e.g. just under "Open All In Tabs", unless you are suggesting I add a listener for the existing "Open All In Tabs" menu item.

Would appreciate it if somebody looked at my existing extension source and added a MR or some sort of psuedo code for it...

https://github.com/nyetwurk/rait/blob/6df7d9485c93ef6c167f46b553a5fbc740262835/background.js#L5

browser.contextMenus.create({
    id: "replace-all-in-tabs",
    title: "Replace All In Tabs",
    contexts: ["bookmark"]	// does not work in chrome, of course.
});

Comment 29

[Rob Wu [:robwu]]

(In reply to Nye Liu from comment #28)

Would appreciate it if somebody looked at my existing extension source and added a MR or some sort of psuedo code for it...

I'm not going to create a PR for your project, but if you need an actual code example I happen to have created an extension that does something special for bookmarks vs bookmark folders:

• menus.onShown that conditionally toggles the visibility of menus:
  https://github.com/Rob--W/bookmark-container-tab/blob/4ea9445cf425bcd47edddf0be2d52258547c128d/background.js#L60-L74
• bookmarks.get call to look up the bookmark and whether it is a folder, and uses menus.update to disable the menu item (the extension uses enabled, but if you want to hide the menu item, use visible instead:
  https://github.com/Rob--W/bookmark-container-tab/blob/4ea9445cf425bcd47edddf0be2d52258547c128d/background.js#L148-L183

Given that the requested functionality can already be implemented, I'm closing this bug.

Comment 30

[Nye Liu]

I still don't see how to create an entirely new item in the left click bookmark menu context.

unction createRootMenuItems() {
    browser.menus.create({
        id: BOOKMARK_MENU_ITEM_ID,
        contexts: ["bookmark"],
        title: browser.i18n.getMessage("open_one_in_container_tab"),
    });
    browser.menus.create({
        id: "firefox-default",
        parentId: BOOKMARK_MENU_ITEM_ID,
        contexts: ["bookmark"],
        title: browser.i18n.getMessage("no_container"),
    });
    browser.menus.create({
        id: "separator-after-no-container",
        type: "separator",
        parentId: BOOKMARK_MENU_ITEM_ID,
        contexts: ["bookmark"],
    });
}

AFAICT browser.menu.create() in the bookmark context can only create items in the right click menu, which is what this ticket is about.

The ticket is not to add a listener to an existing one.

Comment 31

[Nye Liu]

My extension already does exactly what yours does - it adds a menu item to the right click context, but it cannot add anything to the left click context.

If you add my extension and yours you can see they both add items to the right click context, but the left click context remains the same.

This ticket is to allow adding an item to the left click context.

I have tried to add a listener to the "Open all in Tabs" event, but it doesn't seem to work right.

Comment 32

[Rob Wu [:robwu]]

That menu is not a context menu, so I'll reopen and adjust the title to describe the requested feature more accurately.

This feature sounds similar to bug 1808148, as in you both want to register a custom action on a bookmark item that is not navigation.

Without new APIs, there is a hacky way to implement the desired functionality, as follows:

• Use the bookmarks API to create (and/or update) a new bookmark with a unique, somewhat semi-secret/random URL. The URL should ideally not resolve anywhere, e.g. https://fakebookmark.local/bookmarkid#123.

Register a webRequest.onBeforeRequest listener for that fake bookmark URL. Verify that the URL is really the bookmark (i.e. only available to the user, not to websites) and if so, cancel the request (a redirect to a HTTP 204 response would ensure that the user stays on the previous page). And then do your custom action.

Issues with this approach:

• the bookmark appears in the autocompletion of the location bar.
• the bookmark may appear in the history.
• the bookmark may be synced to other devices.

Additional aspects:

• other extensions can read the bookmark and so something with it.

A dedicated API could offer similar functionality, but with fewer issues. I can see pros and cons with a bookmark-based API and a menus-based API.

Comment 33

[Nye Liu]

Exactly this! Thank you.

Comment 34

[kernp25]

Attachment: Bug 1448518 - Support registration of items in bookmarks folders in toolbar. r?robwu

Comment 35

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.