Support WebExtensions bookmark context menus

RESOLVED FIXED in Firefox 59

Status

()

Toolkit
WebExtensions: General
P5
enhancement
RESOLVED FIXED
6 months ago
6 days ago

People

(Reporter: jkt, Assigned: ntim)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {dev-doc-needed})

54 Branch
mozilla59
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: [design-decision-approved][contextMenus] triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Reporter)

Description

6 months ago
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: [...]}
});

Updated

6 months ago
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)
(Reporter)

Comment 3

5 months ago
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)
Sure thing! 

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

Agenda: https://docs.google.com/document/d/1gWszBunGAyOJ_V8_HMECXJuZ4Gd_HTM_M7xjDSwSxeo/edit#
Flags: needinfo?(cneiman)
Created attachment 8885404 [details]
bookmark toolbar.png

Here's one showing the pocket menu
(Reporter)

Comment 7

5 months ago
> 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.
(Reporter)

Comment 8

5 months ago
Created attachment 8885742 [details]
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.
(Assignee)

Comment 10

5 months ago
(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.
(Reporter)

Comment 11

5 months ago
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.
(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.
(Reporter)

Comment 13

5 months ago
(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"
(Assignee)

Updated

5 months ago
Priority: -- → P5

Comment 14

5 months ago
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

Updated

25 days ago
Duplicate of this bug: 1414458

Updated

25 days ago
Blocks: 1215059
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)
(Assignee)

Comment 17

21 days ago
(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)

Updated

21 days ago
Assignee: nobody → ntim.bugs
Status: NEW → ASSIGNED
Summary: Consider support for bookmark context menus → Support WebExtensions bookmark context menus
Comment hidden (mozreview-request)
(Assignee)

Updated

21 days ago
Keywords: dev-doc-needed
(Assignee)

Comment 19

21 days ago
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.
(Assignee)

Comment 20

21 days ago
Try is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2df57b30f99c376cda4296b4f2b902ad6a5c6037
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)
Comment hidden (mozreview-request)
(Assignee)

Comment 23

21 days ago
Fixed this, also it needs the bookmarks permission now.
Flags: needinfo?(ntim.bugs)

Comment 24

21 days ago
mozreview-review
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 hidden (mozreview-request)

Comment 26

21 days ago
mozreview-review
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+

Comment 27

21 days ago
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/09c62da7c2a5
Support WebExtensions bookmark context menus. r=mixedpuppy
(Assignee)

Updated

21 days ago
Depends on: 1419195

Comment 28

20 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/09c62da7c2a5
Status: ASSIGNED → RESOLVED
Last Resolved: 20 days ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59

Updated

19 days ago
Flags: needinfo?(amckay)

Comment 29

9 days ago
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/
(Assignee)

Comment 30

9 days ago
(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.

Comment 31

6 days ago
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.
You need to log in before you can comment on or make changes to this bug.