mozilla-mobile / firefox-ios

Firefox for iOS
Mozilla Public License 2.0
12.14k stars 2.91k forks source link

The account without `username` is displayed and the `email` is displayed as a username on a login attempt #20357

Open abodea opened 3 months ago

abodea commented 3 months ago

Preconditions

Save 2 logins for www.instagram.com 1st login -> add a username, add a password. 2nd login -> Do not add a username, add a password.

Steps to reproduce

  1. Launch FF and connect to FF acount.
  2. Perform a sync > Both Instagram logins are displayed in FFiOS.
  3. Access www.instagram.com.
  4. Tap on the username and then > use saved passwords

    Expected behavior

    Only one login should be displayed (the one that contains the username too).

    Actual behavior

    The account without username is displayed and the email is displayed as a username on a login attempt.

    Device & build information

    • Device: iPhone 15 Pro (17.4.1).
    • Build version: v127 (42000)
    • First seen version: N/A

      Notes

      Please note that on desktop the login without a username is not displayed on the autofill. Attachments:

https://github.com/mozilla-mobile/firefox-ios/assets/42831109/43772c7d-a8e4-41f6-a0b7-3c4cbd6ced87

┆Issue is synchronized with this Jira Task

data-sync-user commented 2 months ago

➤ Matt Lichtenstein commented:

The discrepancy seems to come from the fact that desktop allows passwords to be saved without a username attached to it, whereas mobile does not allow this. Should mobile allow username-less passwords? Or shall we just hide these from the autofill sheet? Norberto Andres Furlan, do you have guidance?

Also, Andrei Bodea, what exactly do you mean by “the email is displayed as a username on a login attempt.”? Do you mean the website is displayed as a username?

data-sync-user commented 2 months ago

➤ Nishant Bhasin commented:

Option a) Filter the logins and not show the ones without username but then what if a website only takes username and sends you a code in email (slack as an example) b) UX input here on what to do with passwords without username those are synced? can we somehow mention and say (no username) and also mention the website somewhere so user knows the passwords are for that particular website

!image-20240611-205733.png|width=504,height=291,alt="image-20240611-205733.png"!

data-sync-user commented 2 months ago

➤ Jeffrey Guerrero commented:

Thanks Nishant Bhasin! Quick question - on option B, when you say “and also mention the website somewhere so user knows the passwords are for that particular website” - shouldn’t this only be populating credentials that fall under the domain listed on top (in this case instagram)?

data-sync-user commented 2 months ago

➤ Matt Lichtenstein commented:

Jeffrey Guerrero

{quote}shouldn’t this only be populating credentials that fall under the domain listed on top (in this case instagram)?{quote}

That is correct, and it also says the website name in the title, below “Use saved password” so this seems to not be an issue. The question really remains whether we should show “(no username)” or something akin to that OR hide these username-less logins from this autofill login sheet.

data-sync-user commented 2 months ago

➤ Norberto Andres Furlan commented:

Ideally, it shouldn’t make sense to display a password that doesn’t have a username. But my understanding is that we can’t a complete saving (always saving username and password), until we update the scripts. Is that correct? Nishant Bhasin Issam Mani Matt Lichtenstein

I think what we can do for now is to display the domain… even though we only display the password related to that domains.. It seems better for me than to show “No username”. What do you think? Jeffrey Guerrero Laura Lopez

data-sync-user commented 2 months ago

➤ Issam Mani commented:

sorry for being late to the party. Yes we do allow username-less entries on Desktop. And we mark them using no username.

!Screenshot 2024-06-13 at 03.20.52.png|width=382,height=238,alt="Screenshot 2024-06-13 at 03.20.52.png"!

!Screenshot 2024-06-13 at 03.21.05.png|width=267,height=114,alt="Screenshot 2024-06-13 at 03.21.05.png"!

data-sync-user commented 2 months ago

➤ Issam Mani commented:

ok I just double checked and the JS script does ping the backend when submission without a username happens. The only case it doesn’t ping is if there is no password. I also removed this check here ( https://github.com/mozilla-mobile/firefox-ios/blob/4cd058fcd2b3879e127877795142955305f4fcce/firefox-ios/Client/Frontend/TabContentsScripts/LoginsHelper.swift#L208 ) for the username. This allows to save records without a username.

!Screen Recording 2024-06-13 at 03.45.58.mov|width=41.666666666666664%,alt="Screen Recording 2024-06-13 at 03.45.58.mov"!

I am guessing it’s only a UX/content question now of how do we want to present logins without a username in the bottomsheet.

FWIW android just shows the password. I think we need to change that there too.

!Screenshot_20240613-034333_Firefox.jpg|width=1080,height=448,alt="Screenshot_20240613-034333_Firefox.jpg"!

data-sync-user commented 2 months ago

➤ Matt Lichtenstein commented:

Issam Mani interesting note, for me, username-less accounts don’t show up in autofill (see attached) (FF v127 on macOS)

!Screenshot 2024-06-13 at 12.59.46 PM.png|width=2554,height=895,alt="Screenshot 2024-06-13 at 12.59.46 PM.png"!

data-sync-user commented 2 months ago

➤ Laura Lopez commented:

Thanks all!

Jeffrey Guerrero since we can save a password without a username, would it make sense on both iOS + Android to flag to a user to save/add a username? Having it say "no username" isn't a great string but it at least alerts the user of what’s missing.

Could we consider:

For transparency, we’re looking to add an informational message bar in Megalist to prompt people to add in a username so there’s a bigger solution on the horizon! I def want a lighter weight fix until Megalist releases.

data-sync-user commented 2 months ago

➤ Andrei Bodea commented:

Verified as fixed on v9000 (43308) with iPhone 15 Pro (17.5.1).

data-sync-user commented 1 month ago

➤ Andrei Bodea commented:

Verified as fixed on v129 (43869) with iPhone 15 Pro (17.5.1).

data-sync-user commented 1 month ago

➤ Matt Lichtenstein commented:

Final resolution:

Username-less logins WILL NOT appear in the list of logins in the login autofill sheet if the sheet is accessed via the username field

Conversely, username-less logins WILL appear in the list when accessed via the password field (no change), and will now report ‘(no username)’ above the ‘****’ password mark instead of the URL hostname

The ‘Used saved password’ accessory view button will no longer appear above the system keyboard when a username field gains focus unless there are saved logins that contain a username