Bug 1519502 will probably break TB as it changes how the childs are addressed, see https://hg.mozilla.org/integration/autoland/rev/904cc7903feb

It is now on autoland.

I think we're mostly okay. Sometimes the menu elements from Lightning don't get the custom element applied, which I don't understand. Only browser_ext_menus.js failed in the Try run I did.

The bindings going away here are are:

• menu.xml#menu-menubar (we don't use it except for toolkit usage)
• menu.xml#menu-menubar-iconic (we don't use it except for toolkit usage)
• menu.xml#menu (we don't use it except for toolkit usage)
• menu.xml#menu-iconic - this one we use (but do not inherit from): https://searchfox.org/comm-central/search?q=menu.xml%23menu-iconic&case=false&regexp=false&path=

Indeed:

TEST-UNEXPECTED-FAIL | comm/mail/components/extensions/test/browser/browser_ext_menus.js | uncaught exception - TypeError: menu.render is not a function at @chrome://global/content/elements/menu.js:148:10

Care to fix it?

The menu.render error can also be seen in mozmill runs, e.g. https://taskcluster-artifacts.net/IjiZ2mhjQOyoPygmo5BxZg/0/public/logs/live_backing.log

There is even this:
JavaScript error: chrome://global/content/elements/menu.js, line 275: TypeError: this.delayConnectedCallback is not a function

The 'delayConnectedCallback' is not defined in the file, but it seems to be something internal to custom elements. If even that is missing, something is thoroughly hosed.

I think I have worked out why there are problems with render and delayConnectedCallback functions. This is odd, but it seems to be true.

The problematic elements are from Lightning via the overlay loader. They all seem to be converted to custom elements (that is, connectedCallback runs), I think. Then the garbage collector gets a hold of them and the custom element stuff is stripped away. I tested this by calling Cu.forceGC() before opening any menu. If a reference is kept, e.g. by logging the element to the console, the menu item appears as normal, if not, it isn't. I've tried this with and without calling document.adoptNode, it makes no difference.

Can any of you Firefox people help with comment 6? We're moving elements from one document to another, so I wonder if there's some crucial reference attached to their original document that should be on the destination document.

(In reply to Geoff Lankow (:darktrojan) from comment #1)

I think we're mostly okay. Sometimes the menu elements from Lightning don't get the custom element applied, which I don't understand. Only browser_ext_menus.js failed in the Try run I did.

I was noticing this too with some menuitems in about:preferences in the WIP at Bug 1500626. Are you using CDATA by any chance? I noticed that the problem went away with Bug 1545962 (though we should track down what's happening regardless).

I've asked about this in https://bugzilla.mozilla.org/show_bug.cgi?id=1500626#c14.

No CDATA, but the problematic nodes are coming from another document. Do you see anything that could be GC related, I'm becoming more convinced that's a part of the problem.

(In reply to Geoff Lankow (:darktrojan) from comment #10)

No CDATA, but the problematic nodes are coming from another document. Do you see anything that could be GC related, I'm becoming more convinced that's a part of the problem.

I think you are right: https://bugzilla.mozilla.org/show_bug.cgi?id=1500626#c26.

I think this is all we need to do once bug 1546467 is landed.

Started a Try run on top of that bug which is currently on autoland.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=88344052b8f93b4443bfb35ec3ad45e4aa2ac821

I've abandoned that Try run, as the mochitest suite has fallen over.

Fixed the test suite, now let's try Try again.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e36f402c2a2d09d246fde1c5ae7eada69c6e87e9

That would have made the stock icons stop working. To make them work we need to add a class menu-iconic.

In this case it's a bit special, since the class will change what kind of menu you have. It's not an "is", but close. So I think it makes sense to be able to see what id that class is applied to.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/58e7ebc66ab9
Adjust usage of the menu-iconic binding which was removed in bug 1519502. r=darktrojan