GNOME Bugzilla – Bug 789018
PopupImageMenuItem setIcon() method does not accept icon as string
Last modified: 2017-10-16 18:53:30 UTC
In js/ui/popupMenu.js function PopupImageMenuItem.setIcon(icon) there is a comment line: // The 'icon' parameter can be either a Gio.Icon or a string. But this seems not to be true. If you give a string as argument to the constructor, like, e.g., new PopupMenu.PopupImageMenuItem("Text", "emblem-system-symbolic"); in a gnome-shell extension, then this fails with Error: Expected type GType for Argument 'type' but got type 'string' Furthermore this new behavior breaks old working extensions. See https://github.com/jaystrictor/gnome-shell-extension-syncthing/issues/30
Created attachment 361645 [details] [review] popupMenu: Fix setting ImageMenuItem's icons via strings Commit 28ca96064bcb added support for setting PopupImageMenuItem's icons via GIcons as well as via strings. However as the check whether an object implements the GIcon interface only works on GObjects, specifying an icon name was broken. Fix that to actually allow both strings and GIcons. (In reply to Jay Strict from comment #0) > Furthermore this new behavior breaks old working extensions. This change was unintentional, so we should fix it. But: The deal extensions get is that they gain access to all internals, in exchange for no API guarantees. So while extensions usually keep working between one stable version and the next, you cannot assume that an update to a newer version will never require any extension code changes - in particular, something working for an "old working extension" does *not* mean that it must never change in future versions.
Review of attachment 361645 [details] [review]: looks fine ::: js/ui/popupMenu.js @@ +409,2 @@ this._icon.gicon = icon; else this allows any other types, including GObjects, to end up in icon_name but, meh
Attachment 361645 [details] pushed as c12da66 - popupMenu: Fix setting ImageMenuItem's icons via strings (In reply to Rui Matos from comment #2) > this allows any other types, including GObjects, to end up in icon_name but, > meh They don't really end up in icon_name, as trying to set the property to anything not a string will throw an exception.