1590362 - Incorrect web address string is displayed for identical username/password entries for origin and subdomain

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: First order -> as described in bug

Affected versions
Nightly 72
Beta 71.0b3

Affected platforms
Windows 10 x64
Ubuntu 18.04

Steps to reproduce

• Launch Firefox
• Go to https://www.facebook.com
• Save a set of credentials, ex: username/password
• Go to https://ro-ro.facebook.com/
• Save the same set of credentials
• Re-open https://www.facebook.com and click on the username field

Expected result
The autocomplete dropdown should show "From this website" under the suggested credentials for www.facebook.com

Actual Result
The "ro-ro.facebook.com" web address string is displayed under the suggested credentials for www.facebook.com
The "From this website" is displayed on ro-ro.facebook.com

Note
This can be reproduced in reverse order as well. Attached video for both cases and logs as well.

Comment 1

[Timea Cernea [:tbabos][inactive]]

Attachment: Console Log ->described scenario

Comment 2

[Timea Cernea [:tbabos][inactive]]

Attachment: Reverse Scenario video

Comment 3

[Timea Cernea [:tbabos][inactive]]

Attachment: Console Logs.zip

Comment 4

[Matthew N. [:MattN]]

Great find! The preferredOrigin argument to LoginHelper.dedupeLogins should be handling this but perhaps the timePasswordChanged resolveBy argument that follows it is the problem, or there is just a bug?

[Tracking Requested - why for this release]: New subdomain login suggestion feature in 71.

Comment 5

[Sam Foster [:sfoster] (he/him)]

• The 2 logins get passed into LoginHelper.dedupeLogins; they are in the order "https://www.facebook.com", "https://ro-ro.facebook.com"
• It uses its internal helper isLoginPreferred to figure out which of the 2 to prefer, using the keys "actionOrigin", "scheme", "subdomain", "timePasswordChanged" to resolve the question.
• actionOrigin and scheme are the same for both
• The subdomain comparison logic looks to see if the 2nd login is a better match to the preferred origin than the 1st login.
• In this case, "ro-ro.facebook.com" is not a better match, so the algorithm continues on to "timePasswordChanged" to break the deadlock.

I looks like we should check the negative case here too. If the 1st origin matches the preferredOrigin and the 2nd doesnt, return false. Its only in the case where either both match the preferredOrigin, or neither do where we should continue.

Comment 6

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1590362 - Prefer matched origin logins when deduping. r?MattN

Comment 7

[Timea Cernea [:tbabos][inactive]]

Hey Matt and Sam,

This issue seems to be fixed in the latest Nightly now.
I tried to find the patch in hope for a Beta uplfit but always got irrelevant bugs for auto-complete.

Last affected build: October 21
First good build: October 22

There are some password fixes landed in that timeframe, maybe one of them could've fix this?
Anyway, just wanted to let you know and see if there are any changes or further actions needed here.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a546ed4e35f
Prefer matched origin logins when deduping. r=MattN

Comment 9

[Sam Foster [:sfoster] (he/him)]

(In reply to Timea Cernea [:tbabos] from comment #7)

Hey Matt and Sam,

This issue seems to be fixed in the latest Nightly now.
I tried to find the patch in hope for a Beta uplfit but always got irrelevant bugs for auto-complete.

There was definitely a bug there, which attachment 9104094 [details] fixes. I'll request a beta uplift for it once it is merged. I was able to reproduce with the steps in comment #0. I'll take a look but I'm not aware of anything that landed recently that would have fixed this.

Comment 10

[Sam Foster [:sfoster] (he/him)]

sorry, getting bugs mixed up with the flag changes. This should be right now. The feature is disabled in <=70, currently in 71 and 72 with this bug.

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/5a546ed4e35f

Comment 12

[Pascal Chevrel:pascalc]

Sam, could you request an uplift to beta? Thanks

Comment 13

[Matthew N. [:MattN]]

(In reply to Sam Foster [:sfoster] (he/him) from comment #9)

(In reply to Timea Cernea [:tbabos] from comment #7)

Hey Matt and Sam,

This issue seems to be fixed in the latest Nightly now.
I tried to find the patch in hope for a Beta uplfit but always got irrelevant bugs for auto-complete.

There was definitely a bug there, which attachment 9104094 [details] fixes. I'll request a beta uplift for it once it is merged. I was able to reproduce with the steps in comment #0. I'll take a look but I'm not aware of anything that landed recently that would have fixed this.

Hi Timea, the bug was sensitive to the last change time of the two logins so ensure that your are consistent with one or the other being older when testing. Can you try repro it again and verify the fix on Nightly?

Comment 14

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 9104094 [details]
Bug 1590362 - Prefer matched origin logins when deduping. r?MattN

Beta/Release Uplift Approval Request

• User impact if declined: In some circumstances, the wrong login is provided as the first suggestion in the password autocomplete menu.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment #0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Change only has impact on sorting and de-duping of logins. Existing logic is otherwise unchanged, we just now also handle this specific case.
• String changes made/needed: None

Comment 15

[Timea Cernea [:tbabos][inactive]]

Reproduced the issue again on affected Nightly 72, Build ID: 20191021215155.

Updated the affected Nightly build to the latest to confirm that the web address strings are correctly displayed now.
Tried it out once more on a new profile with the latest nightly by following the STR and got the same positive results.

Marking it as verified for Nightly 72 (Build ID: 20191028215046) on Windows 10, MacOS 10.15 and Ubuntu 18.04.
Waiting for uplift to Beta.

Comment 16

[Pascal Chevrel:pascalc]

Comment on attachment 9104094 [details]
Bug 1590362 - Prefer matched origin logins when deduping. r?MattN

P1 fix for a new 71 feature, covered by tests and verified by our QA in nightly, uplift approved for 71 beta 6, thanks.

Comment 17

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d2aa52c5e5a4

Comment 18

[Timea Cernea [:tbabos][inactive]]

Verified - fixed on latest Firefox Beta 71.0b6 (20191030153225) - build from Treeherder - on Windows 10, MacOS10.15 and Ubuntu 18.04.