Closed Bug 1650675 Opened 1 year ago Closed 6 months ago

Login CSV import summary dialog

Categories

(Firefox :: about:logins, enhancement, P1)

Desktop
All
enhancement

Tracking

()

VERIFIED FIXED
86 Branch
Tracking Status
firefox86 --- verified
firefox87 --- verified
firefox88 --- verified

People

(Reporter: MattN, Assigned: petcuandrei)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 5 obsolete files)

After a user imports from a CSV file, show a confirmation dialog indicating the number of additions, updates, unchanged exact matches, and errors.

UX Spec: https://mozilla.invisionapp.com/share/R3XX2AF2XN5#/screens/423736091_Import_From_CSV

Perhaps maybeImportLogins can return an array corresponding to the input array

[
  { result: "added", login: {…nsILoginInfo…}},
  { result: "modified", login: {…nsILoginInfo…}},
  { result: "no_change", login: {…nsILoginInfo…}},
  { result: "error_invalid_origin", loginData: {…nsILoginInfo…}},
  { result: "error_invalid_password", loginData: {…nsILoginInfo…}},
]

The CSV code will also have errors that don't apply to a specific login e.g. missing columns, can't read the file, etc.

Assignee: nobody → petcuandrei
Status: NEW → ASSIGNED

Depends on D83193

Attached file Bug 1650675 added mock data (obsolete) —

Depends on D85647

Depends on D87851

Attachment #9171361 - Attachment is obsolete: true
Attachment #9171362 - Attachment is obsolete: true
Attachment #9167443 - Attachment description: Bug 1650675 initial draft for the dialog → Bug 1650675 Import dialog summary

Depends on D85647

Attachment #9184970 - Attachment is obsolete: true

Depends on D85647

Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e281a982040
Login CSV import summary backend r=MattN
https://hg.mozilla.org/integration/autoland/rev/6b9b5c6906b8
Import dialog summary r=jaws,flod
Attachment #9192859 - Attachment is obsolete: true
Flags: needinfo?(petcuandrei)
Attachment #9186089 - Attachment description: Bug 1650675 error dialog draft → Bug 1650675 Import CSV error dialog

Depends on D96101

Attachment #9195137 - Attachment description: Bug 1650675 Detailed import summary → Bug 1650675 Detailed import summary DRAFT
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4bfd23bcf736
Login CSV import summary backend r=MattN
https://hg.mozilla.org/integration/autoland/rev/7ac7236854d4
Import dialog summary r=jaws,flod
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0fa2ae373bb9
Fixed lint error in import-summary-dialog.css. a=lint-fix CLOSED TREE

I have started to test these changes on the latest Nightly 86.0a1 (Build ID: 20210111093448) on Windows 10 x64, macOS 10.15.6 and Linux Mint 20.
There are a few scenarios that I still need to cover before marking this bug as verified fix.

  • However, during testing I have noticed that the "Import Error" dialog is not displayed if trying to import an invalid CSV file (eg: without the password column) or a different type of file (eg: .txt or .json).
    I am not sure if the "Import Error" dialog is implemented in this bug or there is an issue with this dialog? Andrei, can you please confirm if the "Import Error" dialog is implemented and should be displayed?
Flags: needinfo?(petcuandrei)

Import error and import details list are not implemented. I hhave 2 patches for them under various degrees of review.

Flags: needinfo?(petcuandrei)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment on attachment 9195137 [details]
Bug 1650675 Detailed import summary DRAFT

Revision D100639 was moved to bug 1649940. Setting attachment 9195137 [details] to obsolete.

Attachment #9195137 - Attachment is obsolete: true
See Also: → 1687852
Status: REOPENED → RESOLVED
Closed: 7 months ago6 months ago
Resolution: --- → FIXED
See Also: → 1688213
See Also: → 1688653

I have verified this issue using the latest Firefox Nightly 88.0a1 (Build ID: 20210303215027) on Windows 10 x64, Ubuntu 20.04 and macOS 11.1.

  • In order to verify this bug, I have tested the following scenarios:
  1. The “Import Complete” modal is displayed after importing logins from a CSV file and this modal contains the following summary of logins:
    • New logins added:
    • Existing logins updated:
    • Duplicate logins found:
    • Errors:
  2. The “Import Error” modal with the “File Format Issue” message is displayed after importing a non-CSV file. However, based on this mock-up the “Unable to Read File, Make sure you selected a CSV file” error should be displayed in this type of the “Import Error” modal.
  3. The “Import Error” modal with the “Multiple Conflicting Values for One Login” message is displayed after importing a CSV file that contains multiple columns header.
  4. The “Import Error” modal with the “File Format Issue” message is displayed after importing a CSV file with missing columns header.
  5. Also, in the mock-up, there is specified that the “Unable to Read File '' error is displayed when trying to import a file that doesn't have permission to read. I am not sure if this case is also implemented. If this is implemented, can you give more information how we can verify this?

@Sam, @Andrei should I log new bugs for the scenarios described in point 2 and provide more details for the scenario described at point 5?

Flags: needinfo?(petcuandrei)
Flags: needinfo?(sfoster)

(In reply to Simona Rosu [:srosu], Ecosystem QA from comment #21)

  1. The “Import Error” modal with the “File Format Issue” message is displayed after importing a non-CSV file. However, based on this mock-up the “Unable to Read File, Make sure you selected a CSV file” error should be displayed in this type of the “Import Error” modal.

I think "Unable to Read File" is specifically for file i/o problems. If a non-CSV format but otherwise readable file is provided, "File Format Issue" seems like the correct result to me.

  1. Also, in the mock-up, there is specified that the “Unable to Read File '' error is displayed when trying to import a file that doesn't have permission to read. I am not sure if this case is also implemented. If this is implemented, can you give more information how we can verify this?

On a mac or linux system, this case can be reproduced by removing the read permissions on a file, e.g. chmod 266 logins.csv. I think it should also be possible to reproduce by providing a file on a path that becomes unreachable, e.g. a disconnected network drive.

Flags: needinfo?(sfoster)
Flags: needinfo?(petcuandrei)

@Sam, thank you for providing more details. I was able to trigger the “Import Error” modal with the “Unable to read file” error after importing a CSV file that doesn’t have read permission on MacOS and Windows 10 x64. Also, I tried to verify this on Windows 10 x64 and a “Import Logins File” OS pop-up with an error message was displayed after importing the CSV file that doesn’t have any kind of permission. Probably this is an OS limitation.

  • I have attached a screenshot with the pop-up.
  • Based on this and comment 21, I’m marking this issue as Verified Fixed.
Status: RESOLVED → VERIFIED
No longer depends on: 1695880
No longer depends on: 1695882
You need to log in before you can comment on or make changes to this bug.