Ubuntu 10.10 shipped with a new shell on the netbook edition (Unity), which features a panel-based menubar, rather than applications drawing their own menubar (see https://wiki.ubuntu.com/MenuBar). In the next version of Ubuntu (11.04), we are merging the desktop and netbook editions and shipping Unity on desktops too (see http://arstechnica.com/open-source/news/2010/10/shuttleworth-unity-shell-will-be-default-desktop-in-ubuntu-1104.ars). In Ubuntu 10.10, we made some progress in adding support for the menubar in to GTK and QT applications (although I wasn't involved with that work), and the focus for 11.04 is to add support to the remaining pieces. Firefox and Thunderbird are probably the most widely used applications on the desktop now which don't yet support the menubar. In the last couple of weeks, I've been working on an extension which adds support for the menubar. The code can be found here: https://code.launchpad.net/~chrisccoulson/globalmenu-extension/trunk, and screenshots of it in action here: http://people.canonical.com/~chrisccoulson/Screenshot_005.png / http://people.canonical.com/~chrisccoulson/Screenshot_004.png. Jorge Castro blogged about this work when I first got it working (http://castrojo.tumblr.com/post/2177489088/application-menu-making-its-way-to-firefox), and Dao expressed some interest in it. I'd be more than happy to work on getting this in, and maintain it as well. The extension is still really just proof-of-concept for the time being, and whilst it's now suitable for day-to-day use (I'm using it by default on my laptop now), it still misses some important functionality which I will be adding in the new year (these being favicon support in the bookmarks and history menu, and keyboard navigation).

The Mac equivalent is in widget/src/cocoa/nsMenu*. Maybe some of that code can be shared.

Yes, the way to do this is to define USE_NATIVE_MENU in nsWebShellWindow.cpp and implement nsINativeMenuService like we do on OS X.

The extension I've been working on implements an interface very similar to nsINativeMenuService at the moment, but with an important difference, because we need to be able to fall back at runtime if users choose to not run unity, whereas this doesn't seem to be the case on OS X

I'm not sure exactly what UX you want. When Unity is enabled, do you want the Firefox app button to appear (like it does when you disable the menubar using Firefox's toolbar context menu)? And how do you want enabling/disabling Unity to interact with the user enabling/disabling Firefox's menubar?

Whatever decisions you make there, I think implementing nsINativeMenuService will still be the right way to go. We may need to extend it for dynamic switching.

The way the extension currently works is: - It hides the Firefox menubar when the Unity panel service exists - It leaves the Firefox menubar visibility alone when not running in Unity (ie, whatever the users settings are) - It doesn't alter the visibility of Firefox app button at all (ie, it doesn't show it when running in Unity). TBH, this is probably somewhere that I could use some guidance. I don't think we should show the Firefox app button automatically when running in Unity, but I'm not sure what to do with the current toolbar context menu entry, ie: - "Menu bar" doesn't make sense, as it won't ever really show the menubar in Unity, but it would still toggle the Firefox button. - If users choose to enable the Firefox button, should we do anything with the menubar in the panel? I'd also like to point out that we undecorate maximized windows in Unity (we remove the titlebar and put the window controls in the panel. The panel shows the window title [1], which fades to the menubar on mouseover[2]). In the current Firefox design, that would put the Firefox button directly under the Ubuntu home button on the panel (which will open the dash). If the button is drawn in the titlebar in the future, then we'd need to consider that too Using nsINativeMenuService would be trivial for me to do, as the current design is modelled around the OS X implementation anyway [1] - http://people.canonical.com/~chrisccoulson/Screenshot_007.png [2] - http://people.canonical.com/~chrisccoulson/Screenshot_006.png

Oh, I missed a point there when saying the Firefox button would be directly under the Ubuntu home button on the panel - hovering over this button in the panel causes the launcher sidebar to unhide (http://people.canonical.com/~chrisccoulson/Screenshot_008.png)

Do you support dynamic turning on and off of Unity? It seems to me that while Unity is enabled, we simply shouldn't allow the menubar to be hidden, and we shouldn't expose the app button. But this is more for our UI overlords to decide :-)

I don't think we will officially support turning off Unity dynamically (we won't provide a way to do this by default). There will be an option at the login screen to select the default (Unity) experience or a classic GNOME session. However, as Unity is just a compiz plugin, users will be able to unload the plugin at runtime if they install the compiz configuration tool (although, they'll find that they lose their panels if they do that!), and I'm not sure if there's a way to stop those users from doing that, so it's probably a use case we need to support. The current extension I've been working on already handles this though, and will automatically re-enable the Firefox menubar if I unload the Unity plugin. Do you want me to involve our UI guys with this too?

That's up to you.

(In reply to comment #8) > It seems to me that while Unity is enabled, we simply shouldn't allow the > menubar to be hidden, and we shouldn't expose the app button. Yep, keep it simple.

How do keyboard shortcuts to open the menus (eg, Alt+F) work on the Mac? I took a look, but couldn't figure it out. In Unity, I need to handle the keypresses in Firefox (because it has focus) and tell the menu to open. This is currently working well, but I have a feeling that it's all going to fall apart now that bug 607224 landed (and when menu items start using the openedWithKey attribute).

(In reply to comment #12) > How do keyboard shortcuts to open the menus (eg, Alt+F) work on the Mac? I took > a look, but couldn't figure it out. Ctrl-F2 will focus the menu in OS X.

And that doesn't rely on anything in Firefox to make that work? That might be why I couldn't figure it out :) I'm not too sure how to make this work once bug 626825 lands :/

Actually, forget that. Now I've had some sleep and looked at it again, those changes don't really affect my work, but I do need to handle not displaying those menuitems when the menu is not opened by the keyboard. That's pretty trivial for me to do though

Although this feature doesn't made it into Firefox 4 (including more recent Aurora), please note that Canonical have this patch in their Firefox builds. Mozilla/5.0 (X11; Linux i686; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

I'd just like to correct that. We don't have a patch at all, but we do ship an extension

(In reply to Chris Coulson from comment #17) > I'd just like to correct that. We don't have a patch at all, but we do ship > an extension Is a patch expected?

Note that current GNOME 3.4 now supports application menus using GMenuModel in GTK+, and I think current versions of Unity have chosen to adopt that model as well to get unified support for application menus in upstream applications. GNOME 3.4 also supports hiding the menu bar when maximized (see recent comments on bug 513159).

(In reply to Henri Sivonen (:hsivonen) from comment #19) > (In reply to Chris Coulson from comment #17) > > I'd just like to correct that. We don't have a patch at all, but we do ship > > an extension > > Is a patch expected? Hi, sorry I've been quiet on this for a while. Just do people don't think I've forgotten about it, here's the current situation: - We ship an addon in Ubuntu which uses libdbusmenu to export the menubar to the shell - As this is an almost 100% binary addon, I'd really like to kill it as it is becoming no fun to look after. However... - I'm not keen on recommending a runtime dependency on dbusmenu, mainly because we seem to be incapable of preserving a stable ABI. Already, if you wanted to support more than the current Ubuntu version (11.10), then you would need a way to support 2 dbusmenu ABI's from the same Firefox binary. That sucks, and it's completely unsuitable for any ISV wanting to provide applications on Ubuntu. - Ryan Lortie has been working on GMenuModel in glib and getting GMenuModel support in GtkApplication. - GtkApplication has the concept of an application menu (which is rendered as a single button in the panel in gnome shell or as the first item in the window menu if you're not running gnome-shell) and a window menubar (which is rendered in the window in gnome-shell, and which we want to render in the panel in Unity). - I think Ryan has just added GMenuModel support in to Unity, so that we do render window menubars from applications that are using GMenuModel + GtkApplication But, there are some limitations with GMenuModel too... - GtkApplication only supports one window menubar per application, which I find a little strange, as it basically turns the window menubar in to an application menu that is drawn in a window. I'm not sure if this is going to change in the future, but it would be difficult to support if it doesn't (eg, I've got no idea how we would support having different menubars for Thunderbird's main window and the compose window, without monitoring top-level focus and swapping out the menubar when the focus changes). - GtkApplication is only in Gtk3. Ok, that's not a completely insurmountable problem, as it would still be possible to write code to export a window menu object without using Gtk3. - There's currently no way for an application to tell if it's menus are open or not with GMenuModel, which means that we can't deliver popupshowing, popupshown etc events to menupopups. This obviously breaks a significant amount of functionality in the menus, including History, Bookmarks and even the Edit menu. So, I'm not sure how best to proceed at the moment. It seems that there is no ideal way forward

Chris: What about re-writing the global menubar binary add-on with js-ctypes? I know it sort-of side-steps the issue (again), but it might make maintainability more realistic. -Mike

The problem is that we use non-scriptable interfaces, such as nsIWidget (there might be some more too).

What interfaces do you need? Maybe we can find a way to make a stable-enough interface for you.

(In reply to Chris Coulson from comment #21) > But, there are some limitations with GMenuModel too... > > - GtkApplication only supports one window menubar per application GAH! Who do we need to lobby in order to make sure that this doesn't move from "haven't gotten around to writing support for this into the prototype" to "enshrined policy"?

(In reply to Colby Russell :crussell from comment #25) > (In reply to Chris Coulson from comment #21) > > But, there are some limitations with GMenuModel too... > > > > - GtkApplication only supports one window menubar per application > > GAH! Who do we need to lobby in order to make sure that this doesn't move > from "haven't gotten around to writing support for this into the prototype" > to "enshrined policy"? GtkApplication represents an application, not a window; its associated menu represents an application-wide menu. If you want to lobby for the inclusion of a per-window menu, feel free, but Gtk intentionally has a single "application" menu.

Chris, any new about this bug? It would be a great improvement also for webapps.

Well, bug 627699 is fixed or soon-to-be fixed now. AFAIK, this might help.

So, with Ubuntu 10.04 and 11.10 becoming EOL before Firefox 21 is released, and the fact that nobody works on dbusmenu anymore (so the current ABI isn't going to change again in future Ubuntu releases), it's probably reasonable to support this properly now (at least until GMenuModel supports images in menus, which it still does not). I'm attaching a patch which adds support for the Unity menu. It's a big patch - I can split it up in to smaller parts on request, but I'm not really sure what people would prefer. The patch is largely based on the addon we've been shipping for a while, with the exception that it doesn't add any new runtime dependencies, and a few parts have been cleaned up / rewritten now that it doesn't have the limitation of using only XPCOM API's

I've updated this now that bug 781360 has landed, and also removed the accidental bump to the glib requirement.

This is updated for recent imagelib changes, plus it fixes some memory issues and potential crashes. I've been using this for a few weeks now

This has some additional clean ups, some comments and has been updated now that bug 748894 has landed. This is probably at the point where I'd be happy for someone to review it, pending it being split up in to smaller, more manageable patches

Sorry for the churn. This mostly adds some missing includes, removes some unnecessary ones, fixes an assertion and makes it possible to pref the whole thing off (I know we do have a small minority of users who turn our addon off purely because they like to be able to drag bookmarks around in the menu, which is something that the Unity menus can't offer). I'm also working on a mock panel service that will make this fully testable. In the meantime, I promise not to touch this anymore :)

So, I decided this weekend to try to fix livemarks, which don't work properly with this patch (and never have done with the addon that we've been shipping). With this new version, livemarks work as expected. The main changes: - It no longer copies what the cocoa implementation does for attaching bindings to menupopups (see bug 365405) as this doesn't work in all cases. Eg, if you create a popup with document.createElement() and then add it to the menu structure, the bindings will never be attached because it didn't have a current document when the wrapper was created, and nsXPConnect::WrapNative will just return the existing wrapper. - It attaches bindings to menu's as well as menupopup's. Those 2 make livemarks work correctly without modifying anything in browser/components/places/content/browserPlacesViews.js, which assumes that bindings are attached in multiple places (eg, it makes use of menu.open) Some other differences: - I got rid of all the reference counting, as it wasn't really necessary. - There's quite a few stylistic changes and renames. - Some other workarounds have gone (eg, Unity opens all menus on startup and then leaves them open, so the old version had a workaround to ignore this the first time. It turns out though that it only does this for empty menus, so I add a placeholder item to empty menus now). - FuncToGpointer is duplicated in lots of places now, so I added a nsGtkUtils.h and put it there instead.

Karl, it looks like there is still a missing feedback request from you. Would you mind to have a look at? Thanks.

Chris, are you still interested in maintaining this, if we can get it reviewed?

In comment 2 of Bug 973392 Chris Coulson seems to indicate he is working on other projects and has not had time to find someone to replace his work as maintainer of Firefox builds in Ubuntu.

That makes it sound like I've not tried to find somebody. It's quite hard to find someone who has the time to maintain something like Firefox in a distro, and do this sort of stuff too. I was hoping that somebody on the Ubuntu Desktop team would help drive this forwards to be honest. I'd love to be able to spend some time on Firefox again, but that's close to impossible at the moment

Based on the comments in the post "We should keep Firefox as default browser in Ubuntu" <http://benjaminkerensa.com/2013/05/15/we-should-keep-firefox>, I e-mailed Chris (@ ubuntu.com) in May about helping out and got no response. (Colby Russell wrote on 2013 May 17) > ... > I'm writing to say, you know, I'm here. So if you're looking for help, > don't hesitate to contact me. > ...

(In reply to Chris Coulson from comment #41) > That makes it sound like I've not tried to find somebody. It's quite hard to > find someone who has the time to maintain something like Firefox in a > distro, and do this sort of stuff too. I was hoping that somebody on the > Ubuntu Desktop team would help drive this forwards to be honest. I'd love to > be able to spend some time on Firefox again, but that's close to impossible > at the moment Would you be interested in a blog post being written? A call for help maybe we can find a contributor or two who could sit down and learn the ropes from you and then start maintaining the package?

(In reply to Colby Russell :crussell from comment #42) > Based on the comments in the post "We should keep Firefox as default browser > in Ubuntu" <http://benjaminkerensa.com/2013/05/15/we-should-keep-firefox>, I > e-mailed Chris (@ ubuntu.com) in May about helping out and got no response. > > (Colby Russell wrote on 2013 May 17) > > ... > > I'm writing to say, you know, I'm here. So if you're looking for help, > > don't hesitate to contact me. > > ... Sorry, it seems Thunderbird moved your message to my junk folder. I've just had a look for it now :( Is that something you're still interested in?

(In reply to Chris Coulson from comment #44) > Is that something you're still interested in? I'm certainly *interested*, but I'm not in as good of a position to just now be taking this up as I was if we had started in July.

(In reply to Colby Russell :crussell from comment #45) > (In reply to Chris Coulson from comment #44) > > Is that something you're still interested in? > > I'm certainly *interested*, but I'm not in as good of a position to just now > be taking this up as I was if we had started in July. Colby, Are you still interested?

(Private response sent.)

I don't mean to be rude, but may I ask about the status of this? It ended with a private message sent and I'm confused whether or not we're pushing forward with it.

(In reply to Brandon Cheng from comment #49) > I don't mean to be rude, but may I ask about the status of this? It ended > with a private message sent and I'm confused whether or not we're pushing > forward with it. We do not currently have anyone from either our community or the Ubuntu community that has the amount of time needed to take on maintaining this if it is reviewed. We need someone to take ownership of this before we can review it.

(In reply to Benjamin Kerensa [:bkerensa] from comment #50) > We do not currently have anyone from either our community or the Ubuntu > community that has the amount of time needed to take on maintaining this if > it is reviewed. We need someone to take ownership of this before we can > review it. What does maintaining this involve? I can fix bugs related or regressions as it appears in the future, but I might not be fast with getting the patch in. Why does maintaining this involve a maintainer when other bugs don't? Is it expertise in the specific code?

(In reply to Brandon Cheng from comment #51) > (In reply to Benjamin Kerensa [:bkerensa] from comment #50) > > We do not currently have anyone from either our community or the Ubuntu > > community that has the amount of time needed to take on maintaining this if > > it is reviewed. We need someone to take ownership of this before we can > > review it. > > What does maintaining this involve? I can fix bugs related or regressions as > it appears in the future, but I might not be fast with getting the patch in. > > Why does maintaining this involve a maintainer when other bugs don't? Is it > expertise in the specific code? It is familiarity with changes that are happening in Unity each cycle that could require additional patches to continue support for Unity. It would be also nice if such a person was already a contributor to the Ubuntu Mozilla Team.

What do we want to do with this? Would be nice to support Unity?

It would indeed. I think karlt, or someone from GTK Widget might be better to ask, though.

Gnome Shell has a kind of application menu. Does it support only a single menu or can more be added? Does it use GDBusMenuModel? Does that use the same DBus interfaces There is also org.kde.kded.appmenu.xml. Are these converging to any common standard? I wonder whether the GMenuModel showing/hidden notifications reported missing in comment 21 are provided now? I don't know what the preferred framework for cross-desktop-environment menu support is. However, there is a lot of good code here, and if it is useful step in the direction of providing cross-environment support, then we should take this dbusmenu implementation.

Comment on attachment 752135 [details] [diff] [review] Support the panel menubar in Unity (v6) Are menus built during startup or when the menu is opened? I'd like to check we won't have issues such as https://code.google.com/p/chromium/issues/detail?id=86715 Do I understand correctly that CreateNativeMenuBar() will create more-or-less everything even if it will not used? Is there an alternative? Are all Linux distributions going to want to compile this code? If not can a configure switch be added, please? I can't review the browser and toolkit changes. Can these be separated into different patches, please? (Compare https://secure.phabricator.com/book/phabflavor/article/writing_reviewable_code/#many-small-commits) I'm also not clear what the _moz-menubarkeeplocal change in nsWebShellWindow.cpp is about. Can that (and associated changes) be made a separate patch with an explanation, please? There are a number of uses of IsSafeToRunScript() where something happens synchronously or off a queued event depending on the return value. Can this lead to actions getting out of order? Would it be better to always run asynchronously, to keep changes in order? Can the dangling pointer issue in FlushPendingMutations() be addressed by keeping a strong reference to the object or by nulling the pointer (a weak reference)? I'll need to get someone else to review the xbl and content attribute modifications. I don't know whether it is practical to separate that into a different patch, but that would be helpful, if possible. If not we'll have to work out how to separate reviews. >+nsDbusmenuFunctions::Init() >+{ >+#define FUNC(name, type, params) \ Probably worth adding another #undef FUNC in this file for the sake of unified compilation (multiple cpp files appended together). > #define LOAD_LIBRARY(symbol, name) \ I wonder whether this would be easier to read as a function: PRLibrary* LoadLibrary(nsDbusmenuDynamicFunction symbols[], size_t length, const char* name) We've made an effort to close dynamically loaded libraries to aid in leak detection. Compare libcanberra in nsSound. Some libraries don't tolerate being unloaded. If that is the case for libdbusmenu*, then a comment should be added to explain. Similarly for libgio-2.0.so.0, but MOZ_ENABLE_GIO is set by default now, in which case other code already assumes that it is loaded, so a simpler/tidier solution may be to use dlopen with a null filename and dlclose after using dlsym. >+#undef g_dbus_proxy_new_for_bus >+#undef g_dbus_proxy_new_for_bus_finish >+#undef g_dbus_proxy_call >+#undef g_dbus_proxy_call_finish >+#undef g_dbus_proxy_get_name_owner Why are these undefined before they are defined? >+ "com.canonical.AppMenu.Registrar", Could this registrar ever exist if getenv("XDG_CURRENT_DESKTOP") is not "Unity"? If so, where would SetOnline(true) be called? If not, can some of this be skipped if not "Unity"? >+nsWidgetGtk2ModuleCtor() >+{ >+ nsAppShellInit(); >+ nsNativeMenuAtoms::Init(); Can nsNativeMenuAtoms::Init() be delayed until it is known that they will be used? >+ nsAutoPtr<nsMenuBar> mMenuBar; Please add a comment explaining the purpose of this reference on nsWindow.

(In reply to Brandon Cheng from comment #51) > What does maintaining this involve? I can fix bugs related or regressions as > it appears in the future, but I might not be fast with getting the patch in. We need someone to review changes to the code, and watch for bug reports. I can CC this person, so they need not watch the whole Widget:GTK component if they would prefer not to. We need someone responsible for common crashes, leaks, and intermittent test failures. Some tasks may require significant chunks of time to address, but having someone on hand to identify the causes of regressions and revert or disable as appropriate may be enough, if that is you? We also need someone to check the patch applies to and works on mozilla-central or update it, and someone to address review questions and comments before it can be pushed. IIUC Ubuntu are still building with a version of this patch, so may have already done the necessary updating. > Why does maintaining this involve a maintainer when other bugs don't? Is it > expertise in the specific code? There is a lot of new code here, code which I don't have time to fully understand and review, and the code is useful to only one or some DEs or distributions IIUC. People on other systems don't benefit and are unable to test changes. I can do a superficial review of some changes and other reviewers can probably look over parts that I don't know about, but from my perspective, I'm mainly interested in ensuring that there are no regressions on other environments. Someone with more interest in Unity integration is needed.

I /think/ Ubuntu uses an addon for unity integration, now.

(In reply to Mike Hommey [:glandium] from comment #58) > I /think/ Ubuntu uses an addon for unity integration, now. Ubuntu did even at the time of Chris's patch being submitted but I believe that addon is not the greatest so support is preferred lets check with Chris.

We have not heard back from the requester for some time closing this.

I'm interested in this as well, not for Unity but for GNOME's application menu. Any objections to leaving this open?

(In reply to Josh Triplett from comment #61) > I'm interested in this as well, not for Unity but for GNOME's application > menu. Any objections to leaving this open? Would you be willing to support the code? I believe were looking for someone to be responsible for it should their be issues that arise.

(In reply to Karl Tomlinson (:karlt) from comment #56) > We've made an effort to close dynamically loaded libraries to aid in leak > detection. Compare libcanberra in nsSound. Some libraries don't tolerate > being unloaded. If that is the case for libdbusmenu*, then a comment should > be added to explain. AFAIK this is no longer relevant as the leak tools have changed and now ignore memory accessible from objects of static storage duration.

(In reply to Karl Tomlinson (:karlt) from comment #56) > Comment on attachment 752135 [details] [diff] [review] > Support the panel menubar in Unity (v6) > > Are menus built during startup or when the menu is opened? > I'd like to check we won't have issues such as > https://code.google.com/p/chromium/issues/detail?id=86715 > The menubar is built on startup, but menus are built when opened. Also, menus are not updated when closed, to avoid this issue. > Do I understand correctly that CreateNativeMenuBar() will create more-or-less > everything even if it will not used? Is there an alternative? > It creates the nsMenuBar instance, but doesn't populate it with anything. It does also create the DbusmenuMenuitem and DbusmenuServer instances - this can probably be avoided though. > Are all Linux distributions going to want to compile this code? If not can a > configure switch be added, please? > There's no harm in compiling it everywhere - it's a no-op in unsupported environments anyway. I don't mind adding a flag though. > I can't review the browser and toolkit changes. Can these be separated into > different patches, please? (Compare > https://secure.phabricator.com/book/phabflavor/article/ > writing_reviewable_code/#many-small-commits) > Possibly. I'll take a look next week to see if it's possible to split up. > I'm also not clear what the _moz-menubarkeeplocal change in > nsWebShellWindow.cpp is about. Can that (and associated changes) be made a > separate patch with an explanation, please? The "Organise", "Views" and "Import and Backup" buttons in the places window (places.xul) are implemented in a menubar. This extra attribute is there to stop it from being exported as a system menu - although it does actually export correctly, I wasn't sure at the time whether this was desirable. > > There are a number of uses of IsSafeToRunScript() where something happens > synchronously or off a queued event depending on the return value. > Can this lead to actions getting out of order? > Would it be better to always run asynchronously, to keep changes in order? > This has been updated quite a bit in a more recent version of the patch. > Can the dangling pointer issue in FlushPendingMutations() be addressed by > keeping a strong reference to the object or by nulling the pointer (a weak > reference)? > This was solved by adding a simple nsWeakMenuObject type in later versions. > I'll need to get someone else to review the xbl and content attribute > modifications. I don't know whether it is practical to separate that into a > different patch, but that would be helpful, if possible. If not we'll have > to > work out how to separate reviews. > I'll try to figure out if that's possible. > >+nsDbusmenuFunctions::Init() > >+{ > >+#define FUNC(name, type, params) \ > > Probably worth adding another #undef FUNC in this file for the sake of > unified > compilation (multiple cpp files appended together). > Ack. > > #define LOAD_LIBRARY(symbol, name) \ > > I wonder whether this would be easier to read as a function: > PRLibrary* LoadLibrary(nsDbusmenuDynamicFunction symbols[], size_t length, > const char* name) > Yes, possibly. > We've made an effort to close dynamically loaded libraries to aid in leak > detection. Compare libcanberra in nsSound. Some libraries don't tolerate > being unloaded. If that is the case for libdbusmenu*, then a comment should > be added to explain. > I'm not sure actually, I'll give it a try. > Similarly for libgio-2.0.so.0, but MOZ_ENABLE_GIO is set by default now, in > which case other code already assumes that it is loaded, so a simpler/tidier > solution may be to use dlopen with a null filename and dlclose after using > dlsym. > Ack. I don't know what the minimum glib version is these days - it might be the case thay we don't need to use dlopen / dlsym here at all now. > >+#undef g_dbus_proxy_new_for_bus > >+#undef g_dbus_proxy_new_for_bus_finish > >+#undef g_dbus_proxy_call > >+#undef g_dbus_proxy_call_finish > >+#undef g_dbus_proxy_get_name_owner > > Why are these undefined before they are defined? IIRC this was to avoid build failures against glib 2.26 and newer. > > >+ "com.canonical.AppMenu.Registrar", > > Could this registrar ever exist if getenv("XDG_CURRENT_DESKTOP") is not > "Unity"? > Yes - I believe there is some mechanism for using this feature in KDE too. > If so, where would SetOnline(true) be called? > If not, can some of this be skipped if not "Unity"? > In this case, SetOnline(true) is called from OnNameOwnerChanged() when there is a valid owner. > >+nsWidgetGtk2ModuleCtor() > >+{ > >+ nsAppShellInit(); > >+ nsNativeMenuAtoms::Init(); > > Can nsNativeMenuAtoms::Init() be delayed until it is known that they will be > used? > Possibly - I'll investigate doing that. > >+ nsAutoPtr<nsMenuBar> mMenuBar; > > Please add a comment explaining the purpose of this reference on nsWindow. Ack.

(In reply to Chris Coulson from comment #65) > > We've made an effort to close dynamically loaded libraries to aid in leak > > detection. Compare libcanberra in nsSound. Some libraries don't tolerate > > being unloaded. If that is the case for libdbusmenu*, then a comment should > > be added to explain. > > > > I'm not sure actually, I'll give it a try. No need now, thanks. (See comment 64.) > > Similarly for libgio-2.0.so.0, but MOZ_ENABLE_GIO is set by default now, in > > which case other code already assumes that it is loaded, so a simpler/tidier > > solution may be to use dlopen with a null filename and dlclose after using > > dlsym. > > > > Ack. > > I don't know what the minimum glib version is these days - it might be the > case thay we don't need to use dlopen / dlsym here at all now. old-configure.in requires GLib/GIO 2.22, but I suspect the GTK 3.4 requirement may indirectly require a more recent GIO anyway.

Reopening due to recent activity.

:glandium, can you suggest someone to review this patch (attachment 8825469 [details] [diff] [review])?

Comment on attachment 8825469 [details] [diff] [review] New patch from ChriA I don't think Mike intended to submit this for review. Someone else will need to offer to maintain and review. (See https://bugzilla.mozilla.org/show_bug.cgi?id=1297530#c8)

A description of the visual behavior this patch adds support for would be helpful. From the comments it sounded like firefox wasn't properly supporting the global application menu under ubuntu. I have ubuntu 16.04, fx 51, all addons disabled and AFAICT the app menu works the same as it does for various other built-in apps. The build I'm testing came down with a system update, 51.0.1+build2-0ubuntu0.16.04.2. Are these changes already baked into the build I'm testing?

(In reply to Jim Mathies [:jimm] from comment #72) > A description of the visual behavior this patch adds support for would be > helpful. From the comments it sounded like firefox wasn't properly > supporting the global application menu under ubuntu. I have ubuntu 16.04, fx > 51, all addons disabled and AFAICT the app menu works the same as it does > for various other built-in apps. > > The build I'm testing came down with a system update, > 51.0.1+build2-0ubuntu0.16.04.2. Are these changes already baked into the > build I'm testing? Yes, the visual behavior is the unified menu on Ubuntu. Ubuntu already builds this patch as part of their custom Firefox which is why you see it working correctly. We're looking to move this patch into Firefox so that we can build the official Unbuntu Firefox builds.

Thanks for splitting this up. I've not been able to spend a lot of time on this because I've had some other things to work on first. I've got an update locally that fixes that build failure, but it aborts at startup because of the changes in bug 1276669. I need to fix that.

(In reply to Chris Coulson from comment #78) > Thanks for splitting this up. I've not been able to spend a lot of time on > this because I've had some other things to work on first. > > I've got an update locally that fixes that build failure, but it aborts at > startup because of the changes in bug 1276669. I need to fix that. I haven't managed to get this building against nightly. Looks like we made some changes to how nsIContent gets handed around, which will require some changes in the menuing code. Working on that currently.

(In reply to Jim Mathies [:jimm] from comment #79) > (In reply to Chris Coulson from comment #78) > > Thanks for splitting this up. I've not been able to spend a lot of time on > > this because I've had some other things to work on first. > > > > I've got an update locally that fixes that build failure, but it aborts at > > startup because of the changes in bug 1276669. I need to fix that. > > I haven't managed to get this building against nightly. Looks like we made > some changes to how nsIContent gets handed around, which will require some > changes in the menuing code. Working on that currently. I've already fixed this.

Here's the updated version, with a fix to cope with the atom table changes. Sorry, it's the complete version again (I'm really busy with release / rust / cargo stuff atm).

Hey all, is Unity dead? Sounds like this work might have been rendered wontfix. https://arstechnica.com/information-technology/2017/04/ubuntu-unity-is-dead-desktop-will-switch-back-to-gnome-next-year/

We should assume this is dead unless we hear otherwise. (I read there's going to be a community trying to keep Unity alive, but that's not going to be a priority for us, and they're probably not ready to maintain this code.)

We are actively working this with Ubuntu. (We had a call this morning.) We're waiting for them to tell us this isn't needed. As it stands, the LTS will have Unity, and Unity will still be around for at least a year, so we're not ready to close this yet.

now i am

At various times, this issue has discussed how to handle integration with the GNOME3 menu as well. I'd still like to see that. Should that be moved to another issue and linked to this one?

> At various times, this issue has discussed how to handle integration with the GNOME3 menu as well. I'd still like to see that. Should that be moved to another issue and linked to this one? Yes, please.