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

VERIFIED FIXED in mozilla1.9alpha8

Status

()

Toolkit
Password Manager
--
major
VERIFIED FIXED
11 years ago
10 years ago

People

(Reporter: Hussam Al-Tayeb, Assigned: Dolske)

Tracking

({privacy, regression})

Trunk
mozilla1.9alpha8
privacy, regression
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
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
(Reporter)

Comment 1

11 years ago
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
(Reporter)

Updated

11 years ago
Keywords: privacy
(Reporter)

Updated

11 years ago
Flags: blocking-firefox3?
(Reporter)

Comment 2

11 years ago
Can someone else reproduce/confirm this problem or am I the only one who sees this bug?

Comment 3

11 years ago
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

Updated

11 years ago
Blocks: 374723
Severity: normal → major
OS: Linux → All
Hardware: PC → All
Version: unspecified → Trunk

Updated

11 years ago
Assignee: nobody → dolske
Flags: blocking-firefox3? → blocking-firefox3+
Target Milestone: --- → Firefox 3 M8
(Assignee)

Comment 4

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

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?
(Assignee)

Comment 7

11 years ago
(Yes, I was just filling your review queue with requests now, and writing tests for them next :)
(Assignee)

Comment 8

11 years ago
Created attachment 277648 [details] [diff] [review]
Patch for testcases, v.1

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.
(Assignee)

Updated

11 years ago
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+
(Assignee)

Comment 10

11 years ago
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
Last Resolved: 11 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
(Reporter)

Comment 11

11 years ago
Verified fixed.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 12

11 years ago
Created attachment 277972 [details] [diff] [review]
Patch to fix testcase.

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+
(Assignee)

Comment 13

11 years ago
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.