GNOME Bugzilla – Bug 666681
[app-menu] section labels are not displayed in the menu
Last modified: 2012-01-16 18:32:48 UTC
Section labels (as in those you can set with e.g. g_menu_append_section()) are not displayed in the menu rendered by the shell. GNOME Documents has an example of such a label.
Created attachment 204063 [details] [review] RemoteMenu: add support for section labels According to the GIO docs, sections can have labels too. We support them by inserting a non reactive menu item at the beginning of the section. This item is specially flagged to be ignored while processing changed signals from the model (since it does not correspond to any model item) Some code paths of this rely on bug 666580 (in particular, separators can be destroyed more than once in some occasions)
Sorry, I meant bug 665680.
Patch works fine but I see a lot of these exceptions in testing: JS ERROR: !!! Exception in callback for signal: destroy JS ERROR: !!! message = '"No signal connection 3 found"' JS ERROR: !!! lineNumber = '74' JS ERROR: !!! fileName = '"/opt/jhbuild/share/gjs-1.0/signals.js"' JS ERROR: !!! stack = '"_disconnect(3)@/opt/jhbuild/share/gjs-1.0/signals.js:74 ([object Object])@/opt/jhbuild/share/gnome-shell/js/ui/popupMenu.js:943 _emit("destroy")@/opt/jhbuild/share/gjs-1.0/signals.js:124 ()@/opt/jhbuild/share/gnome-shell/js/ui/popupMenu.js:146 wrapper()@/opt/jhbuild/share/gjs-1.0/lang.js:167 ()@/opt/jhbuild/share/gnome-shell/js/ui/popupMenu.js:1104 wrapper()@/opt/jhbuild/share/gjs-1.0/lang.js:167 ()@/opt/jhbuild/share/gnome-shell/js/ui/popupMenu.js:1116 wrapper()@/opt/jhbuild/share/gjs-1.0/lang.js:167 _parent()@/opt/jhbuild/share/gjs-1.0/lang.js:158 ()@/opt/jhbuild/share/gnome-shell/js/ui/popupMenu.js:1741 wrapper()@/opt/jhbuild/share/gjs-1.0/lang.js:167 ([object Object])@/opt/jhbuild/share/gnome-shell/js/ui/panelMenu.js:115 wrapper([object Object])@/opt/jhbuild/share/gjs-1.0/lang.js:167 ()@/opt/jhbuild/share/gnome-shell/js/ui/panel.js:569 wrapper()@/opt/jhbuild/share/gjs-1.0/lang.js:167 ([object _private_Shell_WindowTracker],[object _private_GLib_ParamSpec])@/opt/jhbuild/share/gnome-shell/js/ui/panel.js:544 wrapper([object _private_Shell_WindowTracker],[object _private_GLib_ParamSpec])@/opt/jhbuild/share/gjs-1.0/lang.js:167
That's what the bug 665680 is supposed to fix (the patch, not the bug).
Review of attachment 204063 [details] [review]: ::: js/ui/popupMenu.js @@ +1411,3 @@ + destroy: function() { + for (let i = 0; i < this.separators.length; i++) + this.separators[i].destroy(); You could also be defensive here and set: this.separators = []; after destroying all children.
Created attachment 204101 [details] [review] RemoteMenu: add support for section labels According to the GIO docs, sections can have labels too. We support them by inserting a non reactive menu item at the beginning of the section. This item is specially flagged to be ignored while processing changed signals from the model (since it does not correspond to any model item) ----- Definitely makes sense to clear the array.
Review of attachment 204101 [details] [review]: OK. I do wonder whether the complexity of this code is warranted - can we just recreate the menu from the model each time? But let's land this now.
(In reply to comment #7) > Review of attachment 204101 [details] [review]: > > OK. I do wonder whether the complexity of this code is warranted - can we just > recreate the menu from the model each time? But let's land this now. You mean the parent menu? Uhm, yeah, that could work, but it would be a lot less performant, so I'd say, let's avoid it if possible. There's always time to change later, if anything is wrong.
Attachment 204101 [details] pushed as b087191 - RemoteMenu: add support for section labels