After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 658605 - userMenu: Update the user information if the object is already loaded
userMenu: Update the user information if the object is already loaded
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-09-08 21:13 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2011-09-19 16:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
userMenu: Update the user information if the object is already loaded (1.53 KB, patch)
2011-09-08 21:14 UTC, Jasper St. Pierre (not reading bugmail)
reviewed Details | Review
userMenu: Update the user information if the object is already loaded (1.77 KB, patch)
2011-09-08 22:24 UTC, Jasper St. Pierre (not reading bugmail)
needs-work Details | Review
userMenu: Update the user information if the object is already loaded (1.54 KB, patch)
2011-09-10 03:15 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2011-09-08 21:13:59 UTC
This can happen in a few cases, such as the alternative-status-menu extension.
Comment 1 Jasper St. Pierre (not reading bugmail) 2011-09-08 21:14:01 UTC
Created attachment 196038 [details] [review]
userMenu: Update the user information if the object is already loaded

Any time we get a cached user object from the AccountsService, it will
already be loaded.
Comment 2 Florian Müllner 2011-09-08 21:42:41 UTC
Review of attachment 196038 [details] [review]:

Basically looks fine, suggestion below. You should do the same for UserMenuButton (and possibly EndSessionDialog) though. Also, the commit message's subject line is a bit cryptic (what's "the object"?) ...

::: js/ui/userMenu.js
@@ +194,3 @@
+                                                    Lang.bind(this,
+                                                              this._updateUser));
+        }

_updateUser() already handles the case where is_loaded is false, so you could just add

  this._updateUser();

after connecting the signals (which is consistent with polkitAuthenticationAgent).
Comment 3 Jasper St. Pierre (not reading bugmail) 2011-09-08 22:24:09 UTC
Created attachment 196045 [details] [review]
userMenu: Update the user information if the object is already loaded

Any time we get a cached user object from the AccountsService, it will
already be loaded.



I couldn't exactly use what you said
Comment 4 Emmanuele Bassi (:ebassi) 2011-09-09 07:29:45 UTC
Review of attachment 196045 [details] [review]:

::: js/ui/userMenu.js
@@ +196,3 @@
                                                  Lang.bind(this,
                                                            this._updateUser));
+        this.actor.connect('realize', Lang.bind(this, function() {

don't use ::realize — it doesn't do what you think it does.

the signal to know if an actor is going to be painted — i.e. it's visible and it's on the stage — is notify:mapped.
Comment 5 Florian Müllner 2011-09-09 12:39:19 UTC
Review of attachment 196045 [details] [review]:

(In reply to comment #4)
> the signal to know if an actor is going to be painted — i.e. it's visible and
> it's on the stage — is notify:mapped.

Of course that'll mean that we update too often (e.g. every time the menu is opened rather than just the first time), but it shouldn't matter much. If ::style-changed works as well, it's probably slightly better though (as this.actor is non-reactive, so no :hover or similar style changes) ...

::: js/ui/userMenu.js
@@ +6,3 @@
 const GLib = imports.gi.GLib;
 const Lang = imports.lang;
+const Meta = imports.gi.Meta;

Supposed left-over from trying to use Meta.later_add()
Comment 6 Jasper St. Pierre (not reading bugmail) 2011-09-10 03:15:07 UTC
Created attachment 196159 [details] [review]
userMenu: Update the user information if the object is already loaded

Any time we get a cached user object from the AccountsService, it will
already be loaded.
Comment 7 Florian Müllner 2011-09-19 16:32:45 UTC
Review of attachment 196159 [details] [review]:

Looks OK, though using notify::mapped means that we reload the avatar image way too often.
Comment 8 Jasper St. Pierre (not reading bugmail) 2011-09-19 16:40:33 UTC
Attachment 196159 [details] pushed as 543b29e - userMenu: Update the user information if the object is already loaded