1597142 - bookmark context menu is added in history

Status: RESOLVED WORKSFORME

(WebExtensions :: Frontend, defect, P3)

Description

[Sébastien Règne]

Attachment: testcase.zip

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

• Load attached extension, which do:
  browser.menus.create({
  "contexts": ["bookmark"],
  "onclick": console.log,
  "title": "Test case"
  });
• Inspect (to open Toolbox - Extension);
• Open menu > Library > History;
• Rigth-clic on a Recent History.

Actual results:

The context menu has option "Test case".

Expected results:

The context menu hasn't option "Test case".

Comment 1

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Seba S.]

Hi, regseb!

Thanks for your contribution!

I could reproduce the described behavior in:

Ubuntu 18
Firefox 70.0.1

I assume this is not an issue because as far as I understand, the "Test case" file you observe is the file recently downloaded. Anyway, to make sure this is not the case, I'll add product and component so we can research this.

Please let us know if this issue is reproduced in the latest Nightly edition. You can download it from here: https://nightly.mozilla.org/

If you still have the issue please create a new profile, you have the steps here: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager
support.mozilla.orgsupport.mozilla.org

Once you have all this information, please let us know so we can continue investigating.

Regards,

Comment 3

[Sébastien Règne]

Salut,

Same problem with Firefox Nightly:

• I downloaded Firefox Nightly.
• I loaded the extension testcase.zip (https://bugzilla.mozilla.org/attachment.cgi?id=9109314).
• The context menu of the Recent History has option "Test case".

Comment 4

[Rob Wu [:robwu]]

Reproduced. The top-level "History" item in the left library pane is internally implemented as a virtual query, like a bookmark. Its internal ID (among others) is defined at https://searchfox.org/mozilla-central/rev/652014ca1183c56bc5f04daf01af180d4e50a91c/toolkit/components/places/PlacesUtils.jsm#449-456

We should probably ignore those IDs. Also in the bookmarks API.

Comment 5

[max]

I can not reproduce the issue in Firefox-87 (linux) and actually this is the point of my complains.

What is the reason to prohibit interaction with history items if an extension has "history" and "bookmarks" permissions? My intension is to convert some history items to notes stored in external application (Emacs Org mode). It is disappointing that such feature requires implementation of full fledged interface to search history items and to display the list. Even though I do not like built-in selector for history range, custom item in existing context menu would allow a short path to add desired functionality.

I could agree that, strictly speaking, history items are not bookmarks, but there is no equivalent in history API and in firefox they are presented in the same window. In my opinion, the context menu item could be shared as well if the extension has enough permissions.

Comment 6

[Rob Wu [:robwu]]

(In reply to max from comment #5)

What is the reason to prohibit interaction with history items if an extension has "history" and "bookmarks" permissions?

Prohibition is not a goal. There is bug 1643886 about supporting context menus on history items. Patches are welcome, and I am willing to support/review contributions if you're interested.

custom item in existing context menu would allow a short path to add desired functionality.

I could agree that, strictly speaking, history items are not bookmarks, but there is no equivalent in history API

This functionality is not part of the bookmarks nor history API, but of the menus API. The "bookmarks" (or "history") permission requirement exists because extensions without such permissions cannot make use of the exposed IDs (e.g. bookmarkId).

and in firefox they are presented in the same window. In my opinion, the context menu item could be shared as well if the extension has enough permissions.

Even if the UI is currently overlapping, they are conceptually different.

Comment 7

[Sébastien Règne]

I don't reproduce the bug anymore too in Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:87.0) Gecko/20100101 Firefox/87.0.

Comment 8

[Rob Wu [:robwu]]

The menu is not shown because the internal node.bookmarkGuid field is an empty string, which results in an early return at https://searchfox.org/mozilla-central/rev/b6f52976b562008c9d9ceeda22907e1eda506c8e/browser/components/extensions/parent/ext-menus.js#1188

Comment 9

[max]

I have replied to comment #6 in Bug #1643886 comment #13. I am glad to here that support of history context menu was discussed already.