Closed Bug 386005 Opened 13 years ago Closed 12 years ago

passwords deleted from drop down menu in gmail.com apear to be deleted but are still saved

Categories

(Toolkit :: Password Manager, defect, major)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla1.9alpha8

People

(Reporter: ht990332, Assigned: Dolske)

References

Details

(Keywords: privacy, regression)

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070627 Firefox/3.0a6pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070627 Firefox/3.0a6pre

when I try to delete a password using Shift+delete from the drop down menu ( for example gmail.com ), it appears to be deleted but the password is still saved. To correctly delete the password, I have to go to preferences > password manager and delete it from there.

Reproducible: Always
the following error appears in error console when I try to delete a password from drop down menu in gmail.com

Error: [Exception... "'No logins found for hostname (null)' when calling method: [nsILoginManagerStorage::removeLogin]"  nsresult: "0x8057001e (NS_ERROR_XPC_JS_THREW_STRING)"  location: "JS frame :: file:///usr/lib/firefox/components/nsLoginManager.js :: anonymous :: line 425"  data: no]
Source File: file:///usr/lib/firefox/components/nsLoginManager.js
Line: 425
Keywords: privacy
Flags: blocking-firefox3?
Can someone else reproduce/confirm this problem or am I the only one who sees this bug?
i can confirm this behaviour on Trunk, probably due to the new Password Manager
Mozilla/5.0 (Windows; U; Windows NT 6.0; it; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4

See also Bug 381727 on password removing from preferences

confirming and moving to password manager
Status: UNCONFIRMED → NEW
Component: General → Password Manager
Ever confirmed: true
Keywords: regression
QA Contact: general → password.manager
Blocks: 374723
Severity: normal → major
OS: Linux → All
Hardware: PC → All
Version: unspecified → Trunk
Assignee: nobody → dolske
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: --- → Firefox 3 M8
This had me confused for a while, until I finally checked the docs and found that .splice() returns an array (which makes complete sense in hindsight).

I also noticed that the login sorting wasn't sorting on the correct values, so I fixed that here too.
Attachment #276890 - Flags: review?(gavin.sharp)
Comment on attachment 276890 [details] [diff] [review]
Patch for review, v.1

>Index: toolkit/components/passwordmgr/src/nsLoginManager.js

>     function loginSort(a,b) {
>         var userA = a.username.toLowerCase();
>         var userB = b.username.toLowerCase();

>-        if (a < b)
>+        if (userA < userB)

>-        if (b > a)
>+        if (userB > userA)

Gah, who reviewed this stuff? :)
Attachment #276890 - Flags: review?(gavin.sharp) → review+
(Can we write a test for this?)
Flags: in-testsuite?
(Yes, I was just filling your review queue with requests now, and writing tests for them next :)
This adds some tests for the autocomplete dropdown. I wasn't able to get filtering working (ie, type "t", down arrow, and only see logins starting with "t"). But it works for me doing it manually, so there's probably some subtle key event thing I don't understand... But it tests the problem in this bug, so it's good enough.
Attachment #277648 - Flags: review?(gavin.sharp)
Comment on attachment 277648 [details] [diff] [review]
Patch for testcases, v.1

I can't run these tests to verify, because the countLogins patch doesn't apply. I think it'd be a good idea to wrap the addLogin calls in a try/catch (and check the exception) to avoid needing to restart the browser to re-run the test. r=me assuming they all pass.
Attachment #277648 - Flags: review?(gavin.sharp) → review+
Checked in.

Checking in toolkit/components/passwordmgr/src/nsLoginManager.js;
/cvsroot/mozilla/toolkit/components/passwordmgr/src/nsLoginManager.js,v  <--  nsLoginManager.js
new revision: 1.14; previous revision: 1.13
done
Checking in toolkit/components/passwordmgr/test/Makefile.in;
/cvsroot/mozilla/toolkit/components/passwordmgr/test/Makefile.in,v  <--  Makefile.in
new revision: 1.8; previous revision: 1.7
done
Checking in toolkit/components/passwordmgr/test/test_0init.html;
/cvsroot/mozilla/toolkit/components/passwordmgr/test/test_0init.html,v  <--  test_0init.html
new revision: 1.6; previous revision: 1.5
done
RCS file: /cvsroot/mozilla/toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html,v
done
Checking in toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html;
/cvsroot/mozilla/toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html,v  <--  test_basic_form_autocomplete.html
initial revision: 1.1
done
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Verified fixed.
Status: RESOLVED → VERIFIED
The testcase checked in fails on Linux/Windows. This changes the test to delete logins with the "delete" key instead of the "back_space" key. Tested on OS X and Linux, and I manually confirmed the keyboard behavior on Windows.
Attachment #277972 - Flags: review?(gavin.sharp)
Attachment #277972 - Flags: review?(gavin.sharp) → review+
Checked in.

Checking in toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html;
/cvsroot/mozilla/toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html,v  <--  test_basic_form_autocomplete.html
new revision: 1.3; previous revision: 1.2
done
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.