Open Bug 1577177 Opened 5 years ago Updated 2 years ago

Wrong behavior when master password Log in button is used for the first account

Categories

(Firefox :: about:logins, defect, P3)

Desktop
All
defect

Tracking

()

Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- disabled
firefox70 --- wontfix

People

(Reporter: mboldan, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [passwords:primary-password])

Attachments

(1 file, 2 obsolete files)

Attached image master password1.gif (deleted) —

Affected versions

  • 70.0a1

Affected platforms
*All Windows

  • All Mac
  • All Linux

Prerequisites

  • A master password is set.
  • No logins are saved.

Steps to reproduce

  1. Launch Firefox.
  2. Go to about:logins.
  3. Click on 'Create New Login' button and populate all the fields with valid data.
  4. Click the 'Save' button.
  5. Click the 'Cancel' button from the Master Password pop-up.
  6. Click the Log In button from the master password banner displayed below the URL bar.

Expected result

  • All the entered data at step 3 is available and the Master Password pop-up is displayed.

Actual result

  • All the data is deleted and no log in pop-up is displayed.
  • about:logins page is displayed.

Regression range
I will investigate ASAP if this is a regression.

Blocks: 1572049
No longer blocks: 1556658
Priority: -- → P3

I have confirmed locally that this bug is fixed by the patch in bug 1572478.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE

The issue is still reproducible with the provided steps on the latest Nightly 70.0a1 (2019-08-30) build even if the fix for bug 1572478 landed. Tested on Windows 7 x64, Mac 10.14 and Arch Linux 4.12.

If no logins are saved and a password master is set, when creating the first login the "Password Required" pop-up is displayed after clicking the "Save" button. If the "Cancel" button of the pop-up is clicked, a yellow banner warning is displayed on the top of the page that informs you that you need to enter your master password. If you press the "Log in" button from the banner, the "Create New Login" form is dismissed even if the fields are completed. From the QA point of view, the users might expect that the "Password Required" pop-up is redisplayed and the entered data is lost.

Considering this, I will reopen the issue.

Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Has Regression Range: --- → no

Here is the regression range:

Last Good: 2019-07-18
First Bad: 2019-07-19
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bc0fdcef296f2d429e3f4b5f29e264ffc185163d&tochange=23d4ebd5f8e70bb86f179556572b55655d6e066d

I don't realize which bug may be caused this issue, but I observe that the issue was introduced when the Logins and Passwords form was changed (Firefox Lockwise was introduced).

Has Regression Range: no → yes
Component: Password Manager → about:logins
Product: Toolkit → Firefox
Version: 70 Branch → unspecified
Assignee: nobody → jaws
Status: REOPENED → ASSIGNED

(In reply to Mihai Boldan, QA [:mboldan] from comment #0)

  1. Click the 'Save' button.
  2. Click the 'Cancel' button from the Master Password pop-up.
  3. Click the Log In button from the master password banner displayed below the URL bar.

Expected result

  • All the entered data at step 3 is available and the Master Password pop-up is displayed.

If the user clicks cancel in step 5 then I don't think there should be an expectation that the information they typed is preserved since we need to encrypt their data with their master password and they denied that access.

Perhaps an easier fix would be to show the unsaved changes warning in this case? I don't think it's worth adding extra complexity to fix otherwise.

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #5)

If the user clicks cancel in step 5 then I don't think there should be an expectation that the information they typed is preserved since we need to encrypt their data with their master password and they denied that access.

Yes, but we also aren't obligated to delete their form data at this point either. We can instead just return them back to the form with their values preserved, let them make any changes they want, and click Save thus showing the prompt again.

Perhaps an easier fix would be to show the unsaved changes warning in this case? I don't think it's worth adding extra complexity to fix otherwise.

Implementing this adds extra complexity actually since the event is dispatched asynchronously, thus we can't use cancelable events + event.preventDefault, we would need to have a new listener that could listen for a failure response from the parent JSM.

Attachment #9102646 - Attachment is obsolete: true

Mass removing [skyline] and [passwords:management] from about:logins bugs which are no longer useful.

Whiteboard: [passwords:management] [skyline]

As mentioned above...

Yes, but we also aren't obligated to delete their form data at this point either. We can instead just return them back to the form with their values preserved, let them make any changes they want, and click Save thus showing the prompt again.

UX recommendation:
Expectation - User populated data fields should retain content until the user actively selects Cancel (intentionally backs out of the add new login flow).

What's the plan with this bug?

(In reply to Mike Kaply [:mkaply] from comment #10)

What's the plan with this bug?

The approach wasn't ideal, is for an edge case, was possibly going to be obsoleted by other improvements to authentication on about:logins, and now needs rebasing on bug 1639347.

Do you have a need for it? If so, it would be helpful to save a round-trip and give more context in advance.

Do you have a need for it? If so, it would be helpful to save a round-trip and give more context in advance.

Sorry, should have definitely given more context. You had mentioned this bug in reference to my master password policy work so I wanted to know if I needed to worry about it or go ahead and get my patch reviewed and landed.

You can go ahead and get your patch reviewed and landed.

Assignee: jaws → nobody
Status: ASSIGNED → NEW
Attachment #9103736 - Attachment is obsolete: true
Whiteboard: [passwords:primary-password]
Blocks: 1754024
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: