Closed Bug 1370499 Opened 3 years ago Closed 2 years ago

Support WebExtensions bookmark context menus

Categories

(WebExtensions :: General, enhancement, P5)

54 Branch
enhancement

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: jkt, Assigned: ntim)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, Whiteboard: [design-decision-approved][contextMenus] triaged)

Attachments

(4 files)

We get a lot of requests for context menus to control bookmarks in our containers experiment, this currently isn't possible.

Could we consider:

browser.contextMenus.create({
  id: "work-container",
  title: "Open in work container",
  contexts: ["bookmark"]
});

browser.contextMenus.onClicked.addListener(function(info, tab) {
  console.log(info.bookmark); // {url: "blah.com", tags: [...]}
});
Whiteboard: [design-decision-needed][contextMenus] triaged
Hi Jonathan, this has been added to the agenda for the July 11 WebExtension APIs triage meeting. Would you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Next_Meeting

Agenda: https://docs.google.com/document/d/1MEMC7EyZ2VVInbWod7FD2SFGp36osoxQkhUm5f0aNSs/edit#
Flags: needinfo?(bob.silverberg)
In which places where bookmarks are available did you want to be able to add the context menu? Bookmarks are available in a number of places via a number of mechanisms.
Flags: needinfo?(bob.silverberg)
Currently the places that have the menu I think are legitimate to add to are:

- Bookmarks (file) menu.
- Bookmarks sidebar
- Bookmarks toolbar
- Bookmarks sidebar

I'm not familiar with any other areas that I know of, however these could be added in stages. I'm not really sure on priority.

All of these places have individual bookmark items for web pages.

I wouldn't expect the menu item to appear on the folders or in the popout bookmark manage window (I assume the manage window is very different code base).
(In reply to Jonathan Kingston [:jkt] from comment #3)
> Currently the places that have the menu I think are legitimate to add to are:
> 
> - Bookmarks (file) menu.
> - Bookmarks sidebar
> - Bookmarks toolbar
> - Bookmarks sidebar
> 

Did you accidentally add "Bookmarks sidebar" twice, or were you thinking of something else?

> I'm not familiar with any other areas that I know of, however these could be
> added in stages. I'm not really sure on priority.
> 
> All of these places have individual bookmark items for web pages.
> 
> I wouldn't expect the menu item to appear on the folders or in the popout
> bookmark manage window (I assume the manage window is very different code
> base).

Thanks for the info, Jonathan.

Caitlin, can you please add this to next week's agenda now that we have more information?
Flags: needinfo?(cneiman)
Attached image bookmark toolbar.png
Here's one showing the pocket menu
> Did you accidentally add "Bookmarks sidebar" twice, or were you thinking of
> something else?
> 

Whoops looks like I missed the "Browser action" that shane took a picture of also.
Attached image Selection_714.png
For clarity here is a screenshot of the context menu. I expected to be able to add an item to this somewhere.
Jonathan, from whence does your screenshot originate? When I pull down the Bookmarks menu from the main menu, I don't get any context menus at all for the bookmarks in that list. Right-clicking just opens the bookmark.
(In reply to Bob Silverberg [:bsilverberg] from comment #9)
> Jonathan, from whence does your screenshot originate? When I pull down the
> Bookmarks menu from the main menu, I don't get any context menus at all for
> the bookmarks in that list. Right-clicking just opens the bookmark.

This context menu shows up in the old (non-photon) bookmarks menu, the bookmarks sidebar and the bookmarks toolbar.
Attached image Selection_715.png
Works for me post photon....
I click the new star icon in the top bar and right click on the pane that comes from it.
(In reply to Jonathan Kingston [:jkt] from comment #11)
> Created attachment 8885912 [details]
> Selection_715.png
> 
> Works for me post photon....
> I click the new star icon in the top bar and right click on the pane that
> comes from it.

Right, it works from the Bookmarks button in the toolbar, but does not work from the Bookmarks menu in the main menu, not in photon anyway.
(In reply to Bob Silverberg [:bsilverberg] from comment #12)
> (In reply to Jonathan Kingston [:jkt] from comment #11)
> > Created attachment 8885912 [details]
> > Selection_715.png
> > 
> > Works for me post photon....
> > I click the new star icon in the top bar and right click on the pane that
> > comes from it.
> 
> Right, it works from the Bookmarks button in the toolbar, but does not work
> from the Bookmarks menu in the main menu, not in photon anyway.

Hmm not sure which menu you are speaking of, in Linux both of these work for me:
- Hot dogs > Library > Bookmarks > right click item
- Alt+B "top bookmarks menu"
Priority: -- → P5
Sounds like a good idea and no-one had any real objection in the meeting, so let's do this.
Whiteboard: [design-decision-needed][contextMenus] triaged → [design-decision-approved][contextMenus] triaged
Duplicate of this bug: 1414458
Severity: normal → enhancement
When can we expect this to be available in WebExtensions? How much work it would be in case if I am interested to implement it as my first bug?
Flags: needinfo?(amckay)
(In reply to Michal Stanke (Mozilla.cz) [:MikkCZ][:mstanke] (use needinfo) from comment #16)
> When can we expect this to be available in WebExtensions? How much work it
> would be in case if I am interested to implement it as my first bug?


If you'd like to work on this, the code for the bookmarks context menu is here:

https://dxr.mozilla.org/mozilla-central/source/browser/components/places/content/placesOverlay.xul#95

And the code for the webextension API is here:

https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/ext-menus.js
https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/ext-c-menus.js

You can probably look into bug 1268020 for inspiration.
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
Summary: Consider support for bookmark context menus → Support WebExtensions bookmark context menus
Keywords: dev-doc-needed
The current patch works on: the bookmarks toolbar, the library menu, the bookmarks subview, and the bookmarks menu.

It doesn't work on the bookmarks sidebar because it is in a different frame.
ntim, it seems to me this bug is about having context menus that operate on the bookmark, but the patch is providing activeTab data.  I don't think activeTab data is appropriate in this context.  

If you feel otherwise, I think we need to clarify the functionality this bug is about.
Flags: needinfo?(ntim.bugs)
Fixed this, also it needs the bookmarks permission now.
Flags: needinfo?(ntim.bugs)
Comment on attachment 8930083 [details]
Bug 1370499 - Support WebExtensions bookmark context menus.

https://reviewboard.mozilla.org/r/201266/#review206482

::: browser/components/extensions/ext-menus.js:239
(Diff revision 2)
>          }
>          // Select the clicked radio item.
>          item.checked = true;
>        }
>  
>        item.tabManager.addActiveTabPermission();

We'll need to be sure that bookmark context menus don't get into some of the command handling code, such as getting active tab permission.  Be sure to evaluate what data is getting out here.  e.g. the tab data following this may also be an issue.

::: browser/components/extensions/test/browser/browser_ext_contextMenus.js:475
(Diff revision 2)
> +      await browser.contextMenus.create({
> +        title: "Get bookmark",
> +        contexts: ["bookmark"],
> +      });
> +      browser.test.sendMessage("bookmark-created");
> +      browser.contextMenus.onClicked.addListener(async ({bookmarkId}) => {

add a test that activeTab is not given.  also assert that any tab/page data is not included in the details..
Comment on attachment 8930083 [details]
Bug 1370499 - Support WebExtensions bookmark context menus.

https://reviewboard.mozilla.org/r/201266/#review206576

Thanks!  Lets add a followup bug to address the context menu in the bookmark sidebar
Attachment #8930083 - Flags: review?(mixedpuppy) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/09c62da7c2a5
Support WebExtensions bookmark context menus. r=mixedpuppy
Depends on: 1419195
https://hg.mozilla.org/mozilla-central/rev/09c62da7c2a5
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Flags: needinfo?(amckay)
Will we be able to get a callback on "Open All in Tabs" to implement Tab Replace?

https://addons.mozilla.org/en-US/firefox/addon/tab-replace/
(In reply to Nye Liu from comment #29)
> Will we be able to get a callback on "Open All in Tabs" to implement Tab
> Replace?
> 
> https://addons.mozilla.org/en-US/firefox/addon/tab-replace/

This bug doesn't allow replacing the built-in menu item, but it makes it possible to add a new "Open All and replace current tabs" context menu item, that works like the one in the add-on.
Any chance this can be pushed up to ff58? Seems like a long wait for even the most basic webext functionality for a TON of now broken extensions.
Any update here?
(In reply to Worcester12345 from comment #33)
> Any update here?

This is riding the Firefox 59 train and should be available in release on March 13. :) See comment 32 for documentation.
I see the context menu as displayed in the screenshots for builds before this fix, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.
Flags: needinfo?(ntim.bugs)
Flags: needinfo?(ntim.bugs) → qe-verify-
(In reply to Tim Nguyen :ntim from comment #30)
> (In reply to Nye Liu from comment #29)
> > Will we be able to get a callback on "Open All in Tabs" to implement Tab
> > Replace?
> > 
> > https://addons.mozilla.org/en-US/firefox/addon/tab-replace/
> 
> This bug doesn't allow replacing the built-in menu item, but it makes it
> possible to add a new "Open All and replace current tabs" context menu item,
> that works like the one in the add-on.

I'm new at writing web extensions, can you just write a simple code fragment to add an entry to every bookmark *folder*? I can add an entry to the Bookmarks menu, but not the folders. I can probably figure out the rest.
I see the issue: this only works on right click, not left click in bookmarks menu. Is there a way to trap the LEFT CLICK "Open all in tabs" behavior, or add a new entry?
Blocks: 1448518
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.