"Cancel", "Next", and "Sign In" functioning incorrectly on login or locked screen in "Sign In?" section

Bug #1479014 reported by Wise Melon on 2015-07-28
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu GNOME
Low
Unassigned
gnome-shell (Ubuntu)
Low
Unassigned
Vivid
Low
Unassigned
Wily
Low
Unassigned

Bug Description

[Impact]

There are five patches in this one fix. So there are five bugs which are fixed:

- Bug [1]: If the "Next" button ever gets set to "Sign In", it won't
get reset to next until the next question asked by pam.

- Bug [2]: Normally the user isn't allowed to proceed passed
the username question until they've filled it in.
To ensure this, the authprompt code desensitizes
the next button when the number of characters change to
zero.
Unfortunately it fails to desensitize the next button
up front when the entry starts out empty.

- Bug [3]: Users are not allowed to cancel if the verification has not started yet and they are typing in their username.

- Bug [4]: If the authPrompt gets reset for some other reason in the interim, this would mean that we don't fade the authentication prompt back in when it is needed.

- Bug [5]: The user session will crash when the login screen is reactivated due to the fact that the previous the user verifier has not been cleared.

- Bug [6]: (LP: #752220) We currently only cancel the user verifier on reset if verifying, but that means we don't properly cancel it when asking for a username at the "Not Listed" screen.

These are rather important things to be fixed as they make the login process more confusing and difficult.

How the bugs are fixed:

- Bug [1] fix: Reset "Next" button to "Next" when asking for username.

- Bug [2] fix: Disallow the user to press "Next" when the username field is empty up front.

- Bug [3] fix: authPrompt no longer ignores the request to cancel before a user has started verification and is still typing in their username.

- Bug [4] fix: loginDialog now only skips the fade-in if we have already faded-in. It now also only skips resetting if we have already reset.

- Bug [5] fix: We now make sure that once the user verifier has completed its job (of user verification) that it gets cleared so that it does not get in the way later.

- Bug [6] fix: We now unconditionally cancel if the user verifier has been reset.

[Test Case]

- Reproduce Bug [1]:

On the first login screen (or log out - not lock), click "Sign In?", then before doing anything else click "Sign In", if the button allows you to press it and is not greyed out, then you have successfully reproduced this bug.

- Reproduce Bug [2]:

On the locked screen click "Sign In?", then before doing anything else press "Cancel". If nothing happens, you have successfully reproduced this bug.

- Reproduce Bug [3]:

On the locked screen click "Sign In?", then in the username field, start entering your username (or any text), then instead of pressing "Next", press "Cancel", if nothing happens then you have successfully reproduced this bug.

- Reproduce Bug [4]:

Go to the login screen (or locked screen), reset the authentication prompt before you have reached "VERIFICATION_SUCCEEDED", and if you see that the prompt does not fade back in, then you have successfully reproduced this bug.

 - Reproduce Bug [5]:

After you have logged in, go back to the login screen, if the user session crashes, then you have successfully reproduced this bug.

- Reproduce Bug [6]:

On the login screen go to the "Not Listed" section, and then attempt to cancel. If it does not let you, then you have successfully reproduced this bug.

[Regression Potential]

As these patches have already been put to practical use in the upstream version, and the patches which have been put to use in this version, have originated from those patches, I would say that the risk of regression is minimal.

tags: added: gnome3
Tim Lunn (darkxst) on 2015-07-28
Changed in ubuntu-gnome:
milestone: none → wily
Tim Lunn (darkxst) wrote :

Thank you for taking the time to report this bug and helping to make Ubuntu better. The issue you are reporting is an upstream one and it would be nice if somebody having it could send the bug to the developers of the software by following the instructions at https://wiki.ubuntu.com/Bugs/Upstream/GNOME. If you have done so, please tell us the number of the upstream bug (or the link), so we can add a bugwatch that will inform us about its status. Thanks in advance.

Re-assigning to gnome-shell since that draws the UI, still broken as of gnome-shell 3.17.4

affects: gdm (Ubuntu) → gnome-shell (Ubuntu)
Changed in gnome-shell (Ubuntu):
importance: Undecided → Low
status: New → Triaged
Changed in ubuntu-gnome:
status: New → Triaged
importance: Undecided → Low

Ok, I have now also reported the bug here: https://bugzilla.gnome.org/show_bug.cgi?id=753004

It appears that someone else already reported this bug here: https://bugzilla.gnome.org/show_bug.cgi?id=752739 So my report has been marked as duplicate.

Tim Lunn (darkxst) wrote :

those fixes look fine to backport, if you want to help with that, it would be great!

Just let me know how and I would be happy to help.

Tim Lunn (darkxst) wrote :

have you done any debian packaging? you would need to cherrypick the patches from git (its probably ok to squash the 3 into 1) and then apply to gnome-shell package using quilt. Then test build (using a ppa is probably easiest).

Ok, everything is now going well, and it should be ready shortly.

Ok, all is done, I have created a deb file, although I have not changed the conversioning information, I hope that that is ok? You should be able to change that information if you want to, however I found that so many files contained the current version number, that I was unable to change all of them, so I just decided to leave them.

How do you want me to get you the deb file?

I said deb file, I meant files, there are three of them... Just thought I should clarify before I caused confusion.

I previously said that I have not changed the "conversioning" information, I meant "versioning". Sorry, autocorrect...

Tim Lunn (darkxst) wrote :

use 'dch -i' to create a new changelog entry, that is what changes the version

Then create a debdiff with 'debdiff <your_new>.dsc <current_ubuntu>.dsc' and attach here

Thomas Ward (teward) wrote :

Assigning Importance Low and Status Triaged to match the overall gnome-shell bug (in the in-devel release), for uniformity.

Changed in gnome-shell (Ubuntu Vivid):
importance: Undecided → Low
status: New → Triaged

Just so you know what happened, I found that the base version of the patches, was different to that of the current version. So I had to reverse engineer the patches and manually apply them, although I did also use quilt, but not to apply. So I have now got the debdiff file. And have attached it here. Its compiling was successfully, however as my testing environment broke earlier today, I have been unable to further test it. But it should work.

Thomas Ward (teward) wrote :

For the record: they asked for some help in Ask Ubuntu Chat, and I assisted.

Here's what I did to assist:

(1) Walked them through the use of `quilt`. Stated that they should have each upstream patch as its own in `quilt` because of the Origin dep3 headers, but that can be debated.
(2) Spot-checked their backporting of the patch. They had a couple style errors and indentation on their first bits.
(3) Guided them with the DEP3 headers. Because they're needed.
(4) Stylistic suggestions / revisions to their changelog entry, cause they didn't state what files were added/changed/etc. which is important in a changelog.

Additional things:
(5) BUILD TEST! Built in sbuild with a vivid-amd64 test.

What needs to be done is a runtime test, possibly with this landing in -proposed and then being 'installed' from -proposed as a test to see if it resolves the problems.

(It's also why I was doing some minor triage of the bug, 'cause they asked for help)

@Thomas Ward, thank you for that help.

Thomas Ward (teward) wrote :

@The Unknown (yerenkov-scott):

No problem, glad to help where I can. Besides, you're learning new things, which is the goal so you don't have to rely on others to guide you in the future. That's what the whole 'learning experience' thing is about, and I don't mind helping you learn new things like how to do what we've done here. :)

The attachment "lp1479014_vivid_v1.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch

@Ubuntu Foundations Team Bug Bot,

 Sorry, that was in error that I marked that as a patch. I have now removed the tag, and unmarked it as a patch.

tags: removed: patch
Thomas Ward (teward) wrote :

Um... actually, you shouldn't have done that, yerenkov-scott. This is a debdiff, and a patch, and it needs Ubuntu Sponsors to look at it, especially for SRU. You also need to get this bug into the SRU format. Read https://wiki.ubuntu.com/StableReleaseUpdates#Procedure

tags: added: patch
description: updated
description: updated
description: updated
Tim Lunn (darkxst) wrote :

@Thomas, thanks for helping out yerenkov-scott on this one. As a general note, its usually fine to just use the git headers for cherrypicked patches, no real need for DEP3 headers in that case.

@yerenkov-scott I will sponsor this however it all seems incredibly verbose, particularly your SRU details, try and keep it a bit shorter, there is no real to split it out into seperate bugs, just a single description of the main issue would have been fine. Also you missed the regression potential section

Tim Lunn (darkxst) wrote :

@Thomas, well atleast for GNOME patches, which nearly always have bug links etc in the headers, can't really speak for other projects

@Tim, Ok, I am not sure how to make it shorter, I have done my best to write it in the best way I can, if you can think of a better way of putting it all then please do, but I have done my best. I missed the regression potential section because I am not sure what potential regressions exist, so I wouldn't know what to put in that section because I do not know of any, if you could give me some better advice for what to put in that section then please do.

description: updated
summary: - "Cancel" button does not work at first when selecting user that is not
- listed on login screen
+ "Cancel", "Next", and "Sign In" functioning incorrectly on login or
+ locked screen in "Sign In?" section
Thomas Ward (teward) wrote :

@Tim (darkxst) ACK on that. Note that he *did* have to backport the patches and adjust them, as they didn't cleanly apply apparently, from what he told me in other chats, so I'm not sure the git headers would survive that process. "The Unknown" also has said that he's also unsure of regression potentials.

Note that that I try and avoid desktop packages like the plague, usually, and triage thereof, I'm hesitant about the procedure here.

Also, I'm not actually subscribed to this bug, as I already get server team bugs and several other packages' bugs, as well as USNs and DSAs, and security updates from other sources, so my emails and bug attentiveness levels are already innundated.

I'll remember what you said in the future though.

------
Thomas

@Tim, Ok, well I have tried my best to do the regressions potential section, although I understand that it may not be up to standards, but at least I found something to put in there.

description: updated
description: updated

@Tim, I was recently alerted to the fact that those patches also cause this bug: https://bugzilla.gnome.org/show_bug.cgi?id=753181 There are patches available to fix the bug caused by the other patches, what do you want me to do? Create a new debdiff? Add to the old? Or would this be for a different bug report with a new debdiff applying the patches?

As this is the sort of thing that I would assume goes in the regression potential section, I have included details of it there.

@Tim, or would you prefer to deal with it? Although I would be happy to if you want?

Tim Lunn (darkxst) wrote :

At this point I would suggest you try and get upstream to make a new point release (i.e. 3.16.4) that would be more preferable than pulling in the 6 odd patches that are required to fix this bug!

How should I get them to do that? Do I just file a new bug report or try and get in contact with them? Also, does this mean that the debdiff will be scrapped, or will that still go in?

email gnome-shell list.

For vivid it will need backport so 6 patches, not sure thats worth but if you want to prepare ok, can push though

On 11/08/15 19:41, The Unknown wrote:
> How should I get them to do that? Do I just file a new bug report or try
> and get in contact with them? Also, does this mean that the debdiff will
> be scrapped, or will that still go in?
>

@Tim, ok, I'll make the debdiff, also you refer to 6 patches, but I only see 5, as we have the 3 I have already done, and then 2 in the new bug report, so where's the 6th one? Or are there only 5?

As I could not find the 3rd patch you said you wanted me to add in, I have added the two in the bug report: https://bugzilla.gnome.org/show_bug.cgi?id=753181 There seem to only be 2 extra patches needed to fix the bugs. If there is another patch which needs to be included, please let me know and I'd be happy to do so. I have generated a new debdiff with all 5 patches in it. And also updated the description, although my information on reproducing the last 2 bugs is a little vague because I have not actually experienced them as I would need to test the other patches in a testing environment to get the 2 other new bugs, and I have not. I have now also attached the new debdiff.

description: updated

@Tim, ok, I did not see that one, but I would be happy to add it in and I should be able to give you a new debdiff with it included by the end of the day if not sooner.

@Tim, ok, I have applied that patch, and created a new debdiff with all 6 patches in it and attached it here. Let me know if you need anything else.

description: updated
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gnome-shell - 3.16.4-0ubuntu1

---------------
gnome-shell (3.16.4-0ubuntu1) wily; urgency=medium

  * New upstream release
    - fixes issues with gdm Auth prompt LP: #1479014

 -- Tim Lunn <email address hidden> Sun, 18 Oct 2015 08:43:05 +1100

Changed in gnome-shell (Ubuntu Wily):
status: Triaged → Fix Released
Tim Lunn (darkxst) wrote :

This has been fixed in wily, I don't think its a good idea to sru 7 different patches into vivid at this late stage in the cycle (it will be EOL in a few months). For the most part these are largely cosmetic fixes and clearly some of these patches caused regressions, hence the large number of patches, so its not clear if other patches have been missed.

Changed in gnome-shell (Ubuntu Vivid):
status: Triaged → Won't Fix
Changed in ubuntu-gnome:
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Related questions

Remote bug watches

Bug watches keep track of this bug in other bug trackers.