483516 - Port changes to Stored Cookies tab (sorting, multi-delete confirmation) to Cookie Sites tab

Status: RESOLVED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

1. In bug 481959, Select All (Ctrl+A) was added to both trees of the Cookie Manager. Bug 476175 added a confirmation dialog to the Stored Cookies tab which pops up when an attempt is made to delete multiple cookies. Currently there is no such confirmation for the Cookie Sites tab.
2. Bug 476175 and bug 481958 fixed sorting in the Stored Cookies tab (the first bug fixed the case where cookies are added, the second fixes the displayed sort direction). Both cases are easy to fix for the Cookie Sites tab by applying the same logic.

I'll add a patch for all of the above once bug 481958 is checked in.

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Attachment: proposed patch

Note: The attached patch contains an additional fix for the tree of the Stored Cookies tab: With the updateSelection parameter added, CookieColumnSort() works now like PermissionColumnSort() and restores the selection [1] when the sorting is changed (the second parameter is already passed in from the XUL file calls and always TRUE there).

[1]: First row of the selection only, cf. SortTree() in treeUtils.js.

Comment 2

[[e-mail]]

Comment on attachment 367628 [details] [diff] [review]
proposed patch

Hmm, we ought to explicitly clear the selection when reloading permissions (we "accidentally" do that when reloading cookies because of the filter code).

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2, sr=neil

The new patch is updated to tip and resets the selection upon (re)loading permissions. Re-requesting review just to check that you agree with me on where/when to reset the selection. The way I do it now (in loadPermissions()) is in sync with the cookies case where it is triggered by cookiesTree.treeBoxObject.rowCountChanged(0, -oldCount) which is inside filter() and in turn called by loadCookies(). Alternatively I could move it to the cookieReloadDisplay observer but then it's even harder to see why it's working in the cookies case.

Comment 4

[[e-mail]]

Comment on attachment 369390 [details] [diff] [review]
patch v2, sr=neil

>   permissionsTree.treeBoxObject.view = permissionsTreeView;
>-  PermissionColumnSort('rawHost', false);
>+  SortTree(permissionsTree, permissionsTreeView, permissions,
>+           lastPermissionSortColumn, lastPermissionSortColumn,
>+           !lastPermissionSortAscending);
>+  permissionsTreeView.selection.select(-1);
IMHO this belongs before SortTree. r=me with that fixed.

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v3, r+sr=neil [Checkin: Comment 6]

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 369450 [details] [diff] [review]
patch v3, r+sr=neil
[Checkin: Comment 6]


http://hg.mozilla.org/comm-central/rev/de7070b99471