873235 - sdk/context-menu fails to show menu items when menu has been edited using Menu Editor addon

Status: RESOLVED WONTFIX

(Add-on SDK Graveyard :: General, defect)

Description

[Vikas Saurabh]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:22.0) Gecko/20130513 Firefox/22.0 (Nightly/Aurora)
Build ID: 20130513004017

Steps to reproduce:

(1) Install Menu Editor addon (https://addons.mozilla.org/en-US/firefox/addon/menu-editor/)
(2) Edit some items in context menu using it
(3) Install Offline QR generator(https://addons.mozilla.org/en-US/firefox/addon/offline-qr-generator/)


Actual results:

Offline QR generator shows context menu using sdk/context-menu API which don't show up with edited context menu.


Expected results:

Menu entries added via sdk/context-menu should be available

Comment 1

[Vikas Saurabh]

The console shows following exceptions:
Timestamp: 16-May-13 11:14:18 PM
Error: offline_qr: An exception occurred.
TypeError: this.separator is null
resource://gre/modules/commonjs/sdk/context-menu.js 771
Traceback (most recent call last):
  File "resource://gre/modules/commonjs/sdk/context-menu.js", line 945, in updateItemVisibilities
    this.populate(this.items);
  File "resource://gre/modules/commonjs/sdk/context-menu.js", line 715, in populate
    this.createItem(item, after);
  File "resource://gre/modules/commonjs/sdk/context-menu.js", line 838, in createItem
    this.insertIntoXUL(item, xulNode, after);
  File "resource://gre/modules/commonjs/sdk/context-menu.js", line 771, in insertIntoXUL
    before = this.separator.nextSibling;

	
Timestamp: 16-May-13 11:14:22 PM
Error: offline_qr: An exception occurred.
TypeError: xulNode is undefined
resource://gre/modules/commonjs/sdk/context-menu.js 736
Traceback (most recent call last):
  File "resource://gre/modules/commonjs/sdk/context-menu.js", line 949, in updateItemVisibilities
    this.setVisibility(this.items, popupNode, PageContext().isCurrent(popupNode));
  File "resource://gre/modules/commonjs/sdk/context-menu.js", line 736, in setVisibility
    xulNode.hidden = !visible;

Comment 2

[Vikas Saurabh]

It very well might be a bug/issue with Menu Editor addon which was last release in March'11 and hence might not be keeping up with recent changes in FF. But I think existence of one (buggy) addon shouldn't impact the addon APIs.

Comment 3

[Vikas Saurabh]

The issue for offline QR generator is at https://github.com/catholicon/OfflineQR/issues/6

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Not really knowing how the SDK context-menu module works (and not having any clue how Menu Editor works at all), I'd suspect Menu Editor is just blowing away whatever's in the context menu currently and regenerating it from scratch using whatever it found via the traditional method of overlaying xul elements onto the context menu. Since the SDK (presumably) doesn't use the overlays, the SDK-added items don't exist to Menu Editor.

If that's the case, I'm not quite sure what could be done from the SDK's side. It might be able to detect that the items it has added have been blown away and could possibly re-add them, though they'd still end up at the end of the context menu and menu editor still couldn't mess with them...

Comment 5

[Dave Townsend [:mossop]]

(In reply to Wes Kocher (:KWierso) from comment #4)
> Not really knowing how the SDK context-menu module works (and not having any
> clue how Menu Editor works at all), I'd suspect Menu Editor is just blowing
> away whatever's in the context menu currently and regenerating it from
> scratch using whatever it found via the traditional method of overlaying xul
> elements onto the context menu. Since the SDK (presumably) doesn't use the
> overlays, the SDK-added items don't exist to Menu Editor.

That's probably not true, it's more likely looking at the DOM and tweaking that way, that said Menu Editor has long been a source of issues for other add-ons too because for some strange reason they expect the elements they create to still be where they put them in the future.

> If that's the case, I'm not quite sure what could be done from the SDK's
> side. It might be able to detect that the items it has added have been blown
> away and could possibly re-add them, though they'd still end up at the end
> of the context menu and menu editor still couldn't mess with them...

It's difficult to make this stuff bullet-proof. Whatever you do another extension can potentially break it. However the SDK context-menu API is particularly flaky because it has to do weird things to handle overflow across multiple add-ons. We could make this a lot easier now we ship in Firefox by using a shared module to handle most of the XUL logic.

Comment 6

[Vikas Saurabh]

Interestingly, https://addons.mozilla.org/en-us/firefox/addon/awesome-screenshot-capture-/ is able to show context menu.

I couldn't find it's code online, so checked out the installed files instead. Its not using sdk/context-menu, rather in its lib/ui.js, there is a piece of code which is create/appendChild-ing nodes under doc.getElementById('contentAreaContextMenu'); where doc seems to be fetched from mediator.getMostRecentWindow("navigator:browser").document.

I'm not too sure of the code myself... still very shaky around low level addon APIs, but it feels that something similar would be happening in sdk/context-menu too. Can we get ideas from awesome-screenshot-capture addon to fix this issue?

(In reply to Dave Townsend (:Mossop) from comment #5)
> We could make this a lot easier now we ship in
> Firefox by using a shared module to handle most of the XUL logic.
Apart, from the hack-ey investigation from my side above, this seems like the best way going forward :)

Comment 7

[Vikas Saurabh]

I am not sure of how bugzilla flags work... I'm assuming that needinfo means that some more information about the bug is required, but I am unclear of what is it???

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Vikas Saurabh from comment #7)
> I am not sure of how bugzilla flags work... I'm assuming that needinfo means
> that some more information about the bug is required, but I am unclear of
> what is it???

It's to make sure the person that needs to provide some sort of information knows that they need to provide that information.

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Clean up needinfo from me.

Comment 10

[Dave Townsend [:mossop]]

I'm going to call this wontfix simply because we're not going to support users being able to move add-on context menu items around. I've filed bug 882900 which will likely make us behave better in 90% of the cases that menu editor might break us right now.

Comment 11

[t1m3f0x]

All you have to do is make the sdk give the menuitem an id in the dom. Why won't you do that?

Comment 12

[Brett Zamir]

My add-on is now breaking too and I'm getting complaints because my new version, which uses the SDK as recommended, is conflicting with Menu Editor and Menu Wizard (in the case of Menu Wizard it merely cannot be moved, but in Menu Editor, it disappears). The Menu Wizard creator also indicates I need an ID.

If you won't fix this, then it only makes sense to remove those add-ons from AMO so it doesn't cause conflicts for other add-ons produced in a best practices manner. I hope though that you will instead allow adding an ID to menu items and such through the SDK because apparently a lot of users like to edit their menu items.

Thanks...

Comment 13

[Brett Zamir]

Are you still resolved not to fix this? I'd like to know as I keep getting complaints about my add-on breaking as a result of users using it with these menu editors, despite my using the SDK...

Comment 14

[t1m3f0x]

(In reply to Brett Zamir from comment #13)
> Are you still resolved not to fix this? I'd like to know as I keep getting
> complaints about my add-on breaking as a result of users using it with these
> menu editors, despite my using the SDK...
Tell them to use Menu Wizard(https://addons.mozilla.org/en-US/firefox/addon/s3menu-wizard/) instead of Menu Editor. And please see the cause of the problem Bug 956461.