Cookie Manager: Ctrl+Left-Click to select multiple items is broken in 1.6

RESOLVED FIXED in mozilla1.7alpha

Status

()

RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: weed_richard, Assigned: mconnor)

Tracking

Trunk
mozilla1.7alpha
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

In Cookie Manager, if you use Ctrl+Left-Click to select multiple cookies to remove and then hit Remove Cookie, the display just blanks out (without removing) those lines in the list.  If you close and reopen the window, you see that the cookies were *not* deleted.

If you delete the cookies one at a time, it works fine.  This worked fine in 1.3.x...just noticed when I upgraded to 1.6.

Only tested in Windows 2000.


Reproducible: Always

Steps to Reproduce:
1.  Open Cookie Manager.
2.  Using Ctrl+Left-Mouse-Click, select multiple cookies to remove and hit Remove Cookie.
3.  Close Cookie Manager.
4.  Open Cookie Manager again...you will see that the cookies were *not* removed.

Actual Results:  
The problem occurred.

Expected Results:  
It should have removed the selected cookies.  Also, it should not have just blanked out the lines in the display.
(Reporter)

Comment 1

15 years ago
Important:  To recreate the bug, the items you select must *not* be grouped together in the list.  Example:  If there are 5 cookies, pick 1, 3, and 5 using Ctrl+Left-click.

Comment 2

15 years ago
Same behavior with 1.6 on FreeBSD, i.e. cookie 1 disappears from the list,
cookies 3 and 5 go blank (but remain selected/highlighted?) and none of the
cookies get deleted.  

Hitting "Remove" two more times appears to accomplish the original objective of
removing all three.

Same behavior is occurring with a trunk pull from last night.
-> cookies, and confirming on linux 2004011608
No javascript console messages
Assignee: general → darin
Status: UNCONFIRMED → NEW
Component: Browser-General → Cookies
Ever confirmed: true
QA Contact: general → cookieqa

Comment 4

15 years ago
This is what I'm seeing in the js console when the remove is attempted (.6 and
trunk):

Error: cookies[idx] has no properties
Source File: chrome://communicator/content/wallet/CookieViewer.js
Line: 314

Error: cookies[row] has no properties
Source File: chrome://communicator/content/wallet/CookieViewer.js
Line: 165

Error: cookies[row] has no properties
Source File: chrome://communicator/content/wallet/CookieViewer.js
Line: 167

Comment 6

15 years ago
The Hardware/OS should be All/All - see the comments. And IMO the keyword
regression should be included. This has worked with 1.5.
(Assignee)

Comment 7

15 years ago
-> me
Assignee: darin → mconnor
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → mozilla1.7alpha
(Assignee)

Comment 8

15 years ago
Neil, any idea why rowCountChanged fails here?  it only happens with the
selections aren't contiguous.

also, why did you remove the part of the code that keeps the scroll position?
Created attachment 139658 [details] [diff] [review]
Proposed patch

We're getting spurious tree notifications. The old code used to ignore the
resulting exceptions but these days they propagate out of rowCountChanged :-/
Note that setting selectEventsSuppressed to false merely postpones select
events until it is set back to true again.
Comment on attachment 139658 [details] [diff] [review]
Proposed patch

Well when I say spurious I mean unwanted.
Attachment #139658 - Flags: review?(dwitte)
*** Bug 231977 has been marked as a duplicate of this bug. ***
*** Bug 232799 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Attachment #139658 - Flags: review?(dwitte) → review+

Updated

15 years ago
Attachment #139658 - Flags: superreview?(darin)

Comment 13

15 years ago
Comment on attachment 139658 [details] [diff] [review]
Proposed patch

rs=darin
Attachment #139658 - Flags: superreview?(darin) → superreview+
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 15

15 years ago
*** Bug 251312 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.