GNOME Bugzilla – Bug 731320
"not listed" Login screen for root user is not intuitive
Last modified: 2017-07-07 16:21:37 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.
Created attachment 277996 [details] [review] change to disable-userlist mode When there is no user in the list, change to the prompt mode.
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?
Created attachment 354907 [details] [review] Disable user list when no user
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.
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.
Created attachment 354987 [details] [review] Disable user list when no user
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.
Created attachment 355060 [details] [review] Disable user list when no user
OK, thanks again!