password manager infobar doesn't disappear after clicking remember in some cases

VERIFIED FIXED in mozilla1.9beta3

Status

()

Toolkit
Password Manager
P3
normal
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: asa, Assigned: Dolske)

Tracking

Trunk
mozilla1.9beta3
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 years ago
I visited a page that had an already stored login. the login was prefilled for me but the password manager infobar came down anyway. I logged in and then clicked "remember" on the password manager infobar. the bar didn't go away. 

Error: [Exception... "'This login already exists.' when calling method: [nsILoginManager::addLogin]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  location: "JS frame :: file:///C:/Program%20Files/Minefield/components/nsLoginManagerPrompter.js :: anonymous :: line 344"  data: no]
Source File: file:///C:/Program%20Files/Minefield/components/nsLoginManagerPrompter.js
Line: 344

Tested on today's nightly build on Mac Vista.
I see this bug on flyporter.com. Dolske mentioned a discrepancy between the code that prompts to save the password and the code that decides whether to fill in a form when I asked him about it on IRC.
(Assignee)

Comment 2

11 years ago
There are two issues here:

1) The notification bar could probably have a try/catch wrapped around the button actions so that errors can be gracefully noticed and the bar dismissed. Then again, I don't think we want to hide errors (by silently failing), or it will taken longer to identify bugs.

2) There's code in both addLogin() and _onFormSubmit() [and promptAuth()] dealing with avoiding duplicate logins, and of course they're out of sync now.

The .equals() and .equalsIgnorePassword() methods on nsILoginInfo didn't turn out to be as useful as they seemed, because "equals" can mean different things in different contexts. I'll take a look at all the login comparison code to see how best to handle this.
Assignee: nobody → dolske
(Assignee)

Updated

11 years ago
Duplicate of this bug: 400124
(Assignee)

Updated

11 years ago
Duplicate of this bug: 403160
(Assignee)

Updated

11 years ago
OS: Windows Vista → All
Hardware: Macintosh → All
Target Milestone: --- → Firefox 3 M10
Version: unspecified → Trunk
(Assignee)

Updated

11 years ago
Duplicate of this bug: 407605
(Assignee)

Comment 6

11 years ago
407605 was blocking-firefox3+, so rerequesting here.
Flags: blocking-firefox3?
Priority: -- → P3
Target Milestone: Firefox 3 M10 → Firefox 3 M11

Updated

11 years ago
Flags: blocking-firefox3? → blocking-firefox3+
(Assignee)

Comment 7

11 years ago
Created attachment 293347 [details] [diff] [review]
Patch, v.1

The most likely cause of what was happening here is that the onFormSubmit() code was looking for an existing login, using .equals(). That function looks at form field names (which we don't otherwise use), and so if the current form has different field names, .equals() would return false and we'd display the notification bar. The checks in addLogin() ignored field names, and so adding the login would correctly fail as a duplicate.

This patch fixes that by changing the two methods on nsILoginInfo to .equals() and .matches(), and using the correct one as needed. The first is strict, and the latter loose.
Attachment #293347 - Flags: review?(gavin.sharp)
Attachment #293347 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 8

11 years ago
Checking in toolkit/components/passwordmgr/public/nsILoginInfo.idl;
/cvsroot/mozilla/toolkit/components/passwordmgr/public/nsILoginInfo.idl,v  <--  nsILoginInfo.idl
new revision: 1.4; previous revision: 1.3
done
Checking in toolkit/components/passwordmgr/src/nsLoginInfo.js;
/cvsroot/mozilla/toolkit/components/passwordmgr/src/nsLoginInfo.js,v  <--  nsLoginInfo.js
new revision: 1.6; previous revision: 1.5
done
Checking in toolkit/components/passwordmgr/src/nsLoginManager.js;
/cvsroot/mozilla/toolkit/components/passwordmgr/src/nsLoginManager.js,v  <--  nsLoginManager.js
new revision: 1.25; previous revision: 1.24
done
Checking in toolkit/components/passwordmgr/test/unit/head_storage_legacy_1.js;
/cvsroot/mozilla/toolkit/components/passwordmgr/test/unit/head_storage_legacy_1.js,v  <--  head_storage_legacy_1.js
new revision: 1.7; previous revision: 1.6
done
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Verified fixed using Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008011504 Minefield/3.0b3pre. I verified by visting a number of sites including nytimes, facebook, chicagotribune, and washingtonpost. Also tested on Win Vista using the latest nightly.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

11 years ago
Duplicate of this bug: 414091
(Assignee)

Updated

10 years ago
Depends on: 435531
Product: Firefox → Toolkit

Updated

10 years ago
Blocks: 450438
(Assignee)

Updated

10 years ago
No longer blocks: 450438
You need to log in before you can comment on or make changes to this bug.