livemarkChild metaData never set for place context menus

NEW
Unassigned

Status

()

Firefox
Bookmarks & History
6 years ago
5 years ago

People

(Reporter: martin, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
The Places controller builds metaDatas, related to the current selected places nodes, when displaying the context menu.
One metaData is "livemarkChild". But this one is never setted, I suppose since the "Load Livemarks on demand" plan (bug 613588).

in https://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/controller.js#475

The test to determine if the node is a livemark child, is inside another test, 
"if (PlacesUtils.nodeIsBookmark(node))".

The cause is :
- PlacesUtils.nodeIsBookmark(node) rejects a node with an itemId = -1
https://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/PlacesUtils.jsm#165

- the itemId of a livemark child is always -1
https://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/nsLivemarkService.js#962

Steps to reproduce:

In short, add a new menuitem element in the placesContext menupopup, displayed only for livemark children, and try the context menu.

detailled:
- open scratchpad, and select the Browser option in the Environment menu (so, the pref devtools.chrome.enabled must be enabled)
- copy/past the following code inside scratchpad, and run it

function createNewMenuitem() {
    let menuitem = document.createElement("menuitem");
    menuitem.setAttribute("label", "I'm a Livemark Child :)");
    menuitem.setAttribute("selection", "livemarkChild");
    
    let menupopup = document.getElementById("placesContext");
    if (menupopup)
        menupopup.appendChild(menuitem);
}

createNewMenuitem();

- now try the context menu (right-click) on a livemark child, inside the Personal Toolbar or the Bookmarks Menu.
A menuitem labelled "I'm a Livemark Child :)" should appears.

actual result:
the created menuitem isn't displayed

expected result:
the created menuitem should be displayed

a patch will follow
(Reporter)

Comment 1

6 years ago
Created attachment 633985 [details] [diff] [review]
proposed patch
(Reporter)

Comment 2

6 years ago
Note that the livemarkChild metaData is currently used only in https://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/placesOverlay.xul , for 2 menuitems, and as a livemark child isn't considered as a bookmark for now, another possibility is to remove the usage of this metaData.
But, this solution will annoyed me (for an extension) ;) . Nevertheless, I can find a solution for my extension in this case.
(Reporter)

Updated

6 years ago
Attachment #633985 - Flags: review?(mak77)

Updated

6 years ago
Summary: livemarkChild metaData never setted for place context menus → livemarkChild metaData never set for place context menus
hm, yeah this is clearly a valid concern.
Well, the patch should be updated cause I think in the meanwhile we changed that code http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/controller.js#475

I see that we only use livemarkChild to hide context menu options, though those options are already hidden by the fact those are not bookmarks (both cut and info check itemId), so yeah, your supposition in comment 2 is correct, we can get rid of "livemarkChild" completely.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 633985 [details] [diff] [review]
proposed patch

clearing review both cause should be updated, and cause we should probably get rid of the "livemarkChild" stuff.
Attachment #633985 - Flags: review?(mak77)
You need to log in before you can comment on or make changes to this bug.