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 731320 - "not listed" Login screen for root user is not intuitive
"not listed" Login screen for root user is not intuitive
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: login-screen
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Ray Strode [halfline]
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2014-06-06 06:47 UTC by David Liang
Modified: 2017-07-07 16:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
change to disable-userlist mode (1.60 KB, patch)
2014-06-06 06:48 UTC, David Liang
none Details | Review
Disable user list when no user (2.13 KB, patch)
2017-07-05 06:52 UTC, xiaoguang wang
none Details | Review
Disable user list when no user (2.87 KB, patch)
2017-07-06 02:14 UTC, xiaoguang wang
none Details | Review
Disable user list when no user (2.81 KB, patch)
2017-07-07 06:51 UTC, xiaoguang wang
committed Details | Review

Description David Liang 2014-06-06 06:47:58 UTC
https://bugzilla.novell.com/show_bug.cgi?id=878951

When there is no user in the list, it will be better not to display 'no listed' but asking for user name directly.
Comment 1 David Liang 2014-06-06 06:48:47 UTC
Created attachment 277996 [details] [review]
change to disable-userlist mode

When there is no user in the list, change to the prompt mode.
Comment 2 Michael Catanzaro 2017-01-21 03:55:01 UTC
Comment on attachment 277996 [details] [review]
change to disable-userlist mode

Well this looks like something we should surely land, but unfortunately the patch has no doubt bitrotted in the past couple of years. Interested in updating it?
Comment 3 xiaoguang wang 2017-07-05 06:52:07 UTC
Created attachment 354907 [details] [review]
Disable user list when no user
Comment 4 Michael Catanzaro 2017-07-05 11:04:43 UTC
Review of attachment 354907 [details] [review]:

::: js/gdm/loginDialog.js
@@ +709,3 @@
 
+        // Disable user list when there are no users.
+        if (( disableUserList === false ) && ( this._userListLoaded === true )) {

Please match the style of the surrounding code. We don't leave extra spaces inside the parentheses, we use == instead of ===, and we don't write out the equivalence operator anyway when testing against bool. And the inner parentheses here are unneeded. So this should be:

if (!disableUserList && this._userListLoaded)

@@ +710,3 @@
+        // Disable user list when there are no users.
+        if (( disableUserList === false ) && ( this._userListLoaded === true )) {
+            let items = this._userList._items;

_items is a private member of UserList, so you should add a getter method UserList.items() instead of accessing it here. But you don't really need it. It would be even better to instead add a getter UserList.numItems() that returns the number of items in the list instead. I think it would be reasonable to use Object.keys().length for that. That's a bit convoluted, so it would be good to move it into a method of UserList rather than leaving it here in _updateDisableUserList.
Comment 5 Michael Catanzaro 2017-07-05 11:11:46 UTC
Also, _updateDisableUserList now needs to be called from the user-changed callback if UserList.removeUser() or UserList.addUser() is called. This is really minor, but good to do.
Comment 6 xiaoguang wang 2017-07-06 02:14:15 UTC
Created attachment 354987 [details] [review]
Disable user list when no user
Comment 7 Michael Catanzaro 2017-07-06 15:21:44 UTC
Review of attachment 354987 [details] [review]:

::: js/gdm/loginDialog.js
@@ +714,3 @@
+        // Disable user list when there are no users.
+        if (!disableUserList && this._userListLoaded) {
+            if (this._userList.numItems() == 0)

OK, this looks good now, though I see now we can simplify this condition even further:

if (this._userListLoaded && this._userList.numItems() == 0)
    disableUserList = true;

No need to check the current value of disableUserList, and note we avoid using the braces { } because the condition is only one line now.

I'll commit it if you change that.
Comment 8 xiaoguang wang 2017-07-07 06:51:41 UTC
Created attachment 355060 [details] [review]
Disable user list when no user
Comment 9 Michael Catanzaro 2017-07-07 16:21:32 UTC
OK, thanks again!