383723 - Clearing the sort filter in the Cookies dialog makes the data become unsorted and the selection is lost

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P5)

Description

[(no longer active)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5) Gecko/20070606 GranParadiso/3.0a5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5) Gecko/20070606 GranParadiso/3.0a5

Check out the following steps to reproduce the problem for a description.

Reproducible: Always

Steps to Reproduce:
1. Open Options, go to the Privacy section, and click the Show Cookies button.  The Cookies dialog appears, and the focus is set to the filter text box.
2. Press Esc.  The cookies sorted order by host name is lost.  The selection is also lost.
3. Enter some filter, and then clear it.  The same symptoms appear.
Actual Results:  
The selection and sort status are lost upon clearing a filter.

Expected Results:  
The selection and sort status must be preserved.

about:buildconfig

Build platform
target
i686-pc-cygwin

Build tools
Compiler 	Version 	Compiler flags
$(CYGWIN_WRAPPER) cl 	14.00.50727 	-TC -nologo -W3 -Gy -Fd$(PDBFILE)
$(CYGWIN_WRAPPER) cl 	14.00.50727 	-GR- -TP -nologo -Zc:wchar_t- -W3 -Gy -Fd$(PDBFILE)

Configure arguments
--enable-application=browser --enable-update-channel=beta --enable-optimize --disable-debug --disable-tests --enable-static --disable-shared --enable-svg --enable-canvas --enable-default-toolkit=cairo-windows --enable-update-packaging --with-branding=browser/branding/unofficial

Comment 1

[(no longer active)]

Attachment: Restore the sort order and selection status on clearing the filter

The fix consists of three parts:

1. Save the initial selection state of the cookies tree when the dialog is opened.
2. Restore the sort order on the tree when clearing the input filter *before* restoring open state and selection (otherwise, restoring those two goes wrong.)
3. In the handler for pressing the Escape key on the filter textbox, check to see if the filter is already an empty string, and don't do anything in that case.

Comment 2

[(no longer active)]

Could we get a review on this before alpha6 freeze?  Otherwise we have to
retarget it to the next milestone.

Comment 3

[(no longer active)]

Comment on attachment 267715 [details] [diff] [review]
Restore the sort order and selection status on clearing the filter

Asking mano for a review, since mconnor's queue seems to be too busy.

Comment 4

[Mike Connor [:mconnor]]

bumping to M9, hopefully Mano or myself get to this sooner...

Comment 5

[Mike Connor [:mconnor]]

moving out bugs that don't need to block b1

Comment 6

[(no longer active)]

Comment on attachment 267715 [details] [diff] [review]
Restore the sort order and selection status on clearing the filter

Re-requesting review from mconnor...

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 267715 [details] [diff] [review]
Restore the sort order and selection status on clearing the filter


>+    // Restore sort order
>+    var sortby = this._lastSortProperty;
>+    if (sortby == "") {
>+      this._lastSortAscending = false;
>+      this.sort( "rawHost" );
>+    }
>+    else {
>+      this._lastSortAscending = !this._lastSortAscending;
>+      this.sort( sortby );
>+    }

r=me with the odd whitespace removed

Comment 8

[(no longer active)]

Attachment: Patch for check-in

(In reply to comment #7)
> (From update of attachment 267715 [details] [diff] [review])
> 
> >+    // Restore sort order
> >+    var sortby = this._lastSortProperty;
> >+    if (sortby == "") {
> >+      this._lastSortAscending = false;
> >+      this.sort( "rawHost" );
> >+    }
> >+    else {
> >+      this._lastSortAscending = !this._lastSortAscending;
> >+      this.sort( sortby );
> >+    }
> 
> r=me with the odd whitespace removed

Addressed this issue.  This patch is ready for check-in.

Comment 9

[(no longer active)]

Ready for check-in...

Comment 10

[Reed Loden [:reed]]

Checking in browser/components/preferences/cookies.js;
/cvsroot/mozilla/browser/components/preferences/cookies.js,v  <--  cookies.js
new revision: 1.16; previous revision: 1.15
done

Comment 11

[[:Aleksej]]

Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9b2pre) Gecko/2007112704 Minefield/3.0b2pre

The sort mode seems to be retained, compared to 2007112004. I am not so sure about the selection — it changed to a different site sometimes.

Comment 12

[Kevin Brosnan]

v. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007112704 Minefield/3.0b2pre ID:2007112704