142179 - Cookie Manager: sort "Cookie Sites" should sort by Domain (TLD -> left, as labels, not strings, etc.)

Status: RESOLVED WORKSFORME

Description

[John Villalovos]

Sort order in "Cookie Sites" needs to be done by Top Level Domain (TLD) on down
in the Cookie Manager.

Currently the cookie manager sorts sites alphabetically from left to right.

So "my.yahoo.com" is not located anywhere near "www.yahoo.com".

The sort needs to be done so it sorts by TLD first, then domain, sub-domain, etc...

The current behavior makes it very difficult to tell all the sites in a domain
that may be setting cookies.

Comment 1

[John L. Villalovos]

Created attachment 82367 [details] [diff] [review]
An attempt to fix the problem

This is a patch I wrote.  I did NOT test this because I am not sure how to
compile all of Mozilla :(

I did test the ReverseHostName function and it appeared to work in my client
side Javascript code.

My idea in this patch is to reverse the host name for the RawHost field.  Since
it appears that things get sorted by RawHost but the Host field gets displayed.
 So hopefully this will work.

Thanks,
John Villalovos

Comment 2

[John L. Villalovos]

Created attachment 82369 [details] [diff] [review]
Second attempt to fix problem

I learned how to test my fix :)  After testing discovered bugs.  I have added a
"reverseHost" data field to the Cookie and Permissions structures.

I now sort the columns on these structures.  It seems to work pretty well :)

Feedback appreciated.

Comment 3

[Eric Vaandering (no email)]

*** Bug 131503 has been marked as a duplicate of this bug. ***

Comment 4

[Eric Vaandering (no email)]

Updating platform, OS, and severity.

Would like to see this functionality.

Comment 5

[Eric Vaandering (no email)]

Filed bug 143166 which requests this behavior for image blocking sites as well.

Comment 6

[John Villalovos]

Well a side affect of my patch is that the Image Manager is also sorted this
way.  This is because the Image manager is in reality the Cookie manager.

Comment 7

[Eric Vaandering (no email)]

I rebuilt my local version of mozilla with the latest patch. Seems to work as
advertised (only tested cookies since I don't block images). Can you get it
reviewed and checked in?

I didn't look at the patches for Passwords/Forms. I encrypt that info and the
builds I make never seem to be able to deal with that.

Anyhow, I really like this.

Comment 8

[John Villalovos]

Created attachment 83080 [details] [diff] [review]
Fix part 3

Change from this fix and the previous fix is that now the ReverseHostName
function in nsWalletTreeUtils.js any space or ":" characters and any thing
following the space or ":" characters will be trimmed off.

This way when the Password Manager (SignonViewer.js) calls this function
entries will show up next to each other even if they have stuff like:

my.site.com:80
my.site.com:80 (Text from site)
my.site.com (Text from site)

I've tested this and it appears to work for me.

Comment 9

[[e-mail]]

Aren't ports supposed to be stripped off and not stored?
I'm thinking of bug 142803 here.

Comment 10

[[e-mail]]

Comment on attachment 83080 [details] [diff] [review]
Fix part 3

>+    var workname = hostname.replace(/(:.*$)|(\s+.*$)/g, '')
I know I said this will become unnecessary, but you forgot the ; and you could
have used var workname = hostname.replace(/[:\s].*/, "");

>+    var hostsplit = workname.split( "." );
>+    var newhostname = "";
>+    for ( var i = hostsplit.length - 1; i >= 0; i-- ) {
>+        if ( hostsplit[i] != "" ) {
>+            newhostname = newhostname + hostsplit[i] + ".";
>+        }
>+    }
>+    return newhostname;
Use the reverse and join functions:
return hostname.split(".").reverse().join(".");

Comment 11

[John Villalovos]

Created attachment 91675 [details] [diff] [review]
Patch to fix this bug.  Works :)

Thanks for the comments.  I'll leave the thing that strips off the port number
for now because I'm not sure when they will fix the other bug.

I did take your comments about the javascript and put those in to this new
patch.	Thanks for the comments because I know very little about javascript.

Comment 12

[John Villalovos]

Created attachment 91677 [details] [diff] [review]
Got rid of .orig file

I had left a .orig file lying around in the previous patch :(

Comment 13

[Stephen P. Morse]

*** Bug 160632 has been marked as a duplicate of this bug. ***

Comment 14

[Stephen P. Morse]

Comment on attachment 91677 [details] [diff] [review]
Got rid of .orig file

r=morse
shouldn't patches labelled first-attempt and second-attempt be marked as
obsolete?

Comment 15

[John L. Villalovos]

I made previous patches obsolete

Comment 16

[Darin Fisher]

-> suresh

Comment 17

[benc]

Tried to improve searchability of summary b/c of latest dupe...

BTW, yes, please keep the Image Manager bug separate, b/c they do have different
QA and are in different compoennts. (You can make the Image Manager bug depend
on this bug if you want).

Comment 18

[benc]

*** Bug 202044 has been marked as a duplicate of this bug. ***

Comment 19

[[e-mail]]

mvl: what do you think of this one?

the patch here looks ok, although we can remove the bit about truncating port,
because ports in cookies were ditched long ago. ;)

actually i don't like the regexps, because wallet is slow enough already without
having extra help. your tree-in-C++ idea sounded a bit enthusiastic but maybe we
could push the sorting to C++? so we could throw a sort order to the enum (or
array, whatever, since we'll need to re-roll it b/c of frozen interfaces) and
have it call up nsVoidArray's sort. maybe this needs benchmarking at some point
to see if it actually buys anything, although from previous tests we know
sorting a voidarray is uberfast - we have no idea about JS though.

Comment 20

[John Villalovos]

The current patch shouldn't be adding to much extra time to the display.  Pretty
much an order N increase because the values get reformatted before being sorted.
 But yes it is slow with a lot of cookies so if the builtin Javascript sort
procedure could be faster that would be nice :)

And yes the part about ports looks like it could be removed since I no longer
see ports being stored in the cookie information.  But the underlying code here
was also being used by the patch for the Password manager and Image manager. 
And hopefully in the future by the Popup manager.  So I don't know if any of
those store ports off the top of my head.

But it would be nice if this was actually checked into the code sometime.  I'm
guessing that this would provoke some positive responses from people if it was
changed.

Comment 21

[[e-mail]]

heh, apologies for not elaborating further. I agree we should support this, it's
a good rfe... my question is how, exactly. wallet needs a really good whacking
at some point, and we may have a better way to fix this... I just want to see
things get better before they get worse, and slowing down something that's
already slow is probably not a good thing. ;) (of course, benchmarking would
allow us to decide more easily)

and yeah, the permissions backend does have the capability to store ports, but
that feature is semi-broken so we're considering either a) fixing it or b)
removing it. until then we may want to keep the port-handling string fu you've got.

see bug 202131 about speeding up the sorting.

Comment 22

[Urban A. Haas]

It would be a nice enhancement to have a general purpose class to do this type
of sorting. Many other interfaces could benefit from it, like history.

Comment 23

[Ray]

What's the status on this one? I find that this is one of the feature I miss
most about Mozilla. It seems to have been open for about 18 months now - is this
going in any time soon?

Comment 24

[Shankar Unni]

By the way - whoever implements this, remember to *not* reverse the order of
components for plain IP addresses (those need to sort by first byte first, as
they do today). Only host*names* should be reversed.

Comment 25

[Jeremy M. Dolan]

> IP addresses (those need to sort by first byte first, as they do today)

I suspect current behavior is to simply sort IP addresses as if they were a
string. This would mean you get the "incorrect" order:

1.5.6.7
10.5.6.7
2.5.6.7

Or something similar (10 would probably come first, actually). This is a very
minor issue and if it's much additional work, can probably be skipped for now.

Does this just need a superreview? I just manually cleaned out my cookies, and
had they been sorted better, I would have saved quite a bit of time.

Comment 26

[[e-mail]]

Unfortunately the patch no longer works; furthermore it doesn't deal with IP
addressed cookies as was pointed out in comment 24.

Comment 27

[Shankar Unni]

> I suspect current behavior is to simply sort IP addresses as if 
> they were a string. 

> 1.5.6.7
> 10.5.6.7

This is not as bad as it seems - at least all the 1.5.*.*'s will be contiguous,
even if the last two octets are "incorrectly sorted" (from a numeric point of view).

The whole point of special sorting of cookies is to get cookies from the same
domain near each other so that we don't have to scrounge around for them.

(Of course, if the cookies from a site come as a mixture of hostname and
IP-address-based cookies, then we have problems :-).

Comment 28

[Ray]

Just my 2 cents: I don't care about ip addresses because I don't know what sites
they are anyway. Once you go to IP6, then it becomes even more difficult. The
average user doesn't even know anything about dot notation, they probably think
it's just a special name. 

But here something? How about organizing the cookies and the site lists in a
tree structure instead of a flat list? I'd be able to turn enable/disable
cookies based on the domain.

Comment 29

[Kim]

a) when the TLD changes, suggest inserting a half-line blank
space, so that all the cookies under the same TLD can be
easily seen / grasped as a visual unit.


b) bug # 236133 is related, it requests cookies also be able
to be sorted by date of creation, by date of last access,
by number of updates, etc.

Comment 30

[Mike Connor [:mconnor]]

-> me, YACMB

why does the IP addressed sites sort order really matter?  I mean, sure it'd be
nice, and if there's a rational way of doing it, then cool, but I don't think
the sort that happens with this method is worse than what we currently have.

Comment 31

[Urban A. Haas]

The reason the IP addressed sites being sorted is to identify cookies from
similiar sites. The way it is now, cookies from different hosts under the same
domain are all over the place. I believe the idea is, even under IP address,
having them together is good.

Although, I also believe some people were making a case for other parts of the
browser using the same sort code, so something like a password manager having IP
sites organized correctly might make a big difference over say, cookies.

Comment 32

[R.K.Aa.]

*** Bug 240028 has been marked as a duplicate of this bug. ***

Comment 33

[benc]

Get domain sorting done first. For IP address support, you will need to support
IPv6 and IPv4, so you can't sort them correctly w/o code that identifies the
literals, so just focus on one thing at a time. 

From glancing at the patch, it seems that this is implemented w/in cookie
manager. I was thinking the other day that a domain sorting function might be
useful elsewhere as well, in history, bookmarks, email, etc.

Comment 34

[toorffej]

Has there really been no activity since April?  Is there some other bug I should
be reading instead?

This is a real problem for me, since I manage cookies aggressively.  Not being
able to see: 1) cookies by domain and 2) most recent cookies is a real hindrance.

I'll test out the latest patch file.

Given that this was opened over 2 years ago, perhaps you could implement
something now, and figure out a more general solution later?

Comment 35

[Mike Connor [:mconnor]]

its an enhancement, and its for Seamonkey.  My primary focus, right now, is with
Firefox and finishing off 1.0.  If someone else wants to take a stab at it,
great, otherwise I'll get to it at some point.

Comment 36

[toorffej]

I've created an patch that adds the cookie sorting behavior for Firefox.

Bugzilla keeps telling me I have not specified the file to attach.  I don't
understand this: I've made sure to accept cookies, turned off Privoxy, and used
the 'Browse' button to specify the file to be attached.

So I'll have to add it inline here.  Sorry about that.

-----------------------------------
This is my first patch, so I'll describe the process in case I did it wrong.

First, I grabbed the Firefox source tarball for 0.9.3, and unpacked it.

Then, I dropped down into ./mozilla and ran "gmake -f client.mk checkout".  This
is supposed to fetch me the latest source.

Back up one dir, and I ran "patch -p0 < cookiesorter.patch"

Back down to ./mozilla, and ran "gmake -f client.mk build".  The resulting
dist/bin/firefox now has the cookie sorting behavior.

Here is the patch file:
--------------------------------------------------------------
diff -r -c
./mozilla/browser/components/cookieviewer/content/CookieExceptions.xul
./mozilla.new/browser/components/cookieviewer/content/CookieExceptions.xul
*** ./mozilla/browser/components/cookieviewer/content/CookieExceptions.xul	Mon
Nov  3 22:02:34 2003
--- ./mozilla.new/browser/components/cookieviewer/content/CookieExceptions.xul
Mon Sep 27 18:59:06 2004
***************
*** 61,67 ****
         onselect="PermissionSelected();">
     <treecols>
       <treecol id="siteCol" label="&treehead.sitename.label;" flex="3"
!                   onclick="PermissionColumnSort('rawHost', true);"
persist="width"/>
       <splitter class="tree-splitter"/>
       <treecol id="statusCol" label="&treehead.status.label;" flex="1"
                    onclick="PermissionColumnSort('capability', true);"
persist="width"/>
--- 61,67 ----
         onselect="PermissionSelected();">
     <treecols>
       <treecol id="siteCol" label="&treehead.sitename.label;" flex="3"
!                   onclick="PermissionColumnSort('reverseHost', true);"
persist="width"/>
       <splitter class="tree-splitter"/>
       <treecol id="statusCol" label="&treehead.status.label;" flex="1"
                    onclick="PermissionColumnSort('capability', true);"
persist="width"/>
diff -r -c ./mozilla/browser/components/cookieviewer/content/CookieViewer.js
./mozilla.new/browser/components/cookieviewer/content/CookieViewer.js
*** ./mozilla/browser/components/cookieviewer/content/CookieViewer.js	Sun Apr 25
14:11:57 2004
--- ./mozilla.new/browser/components/cookieviewer/content/CookieViewer.js	Mon
Sep 27 18:58:58 2004
***************
*** 113,118 ****
--- 113,119 ----
    this.isDomain = isDomain;
    this.host = host;
    this.rawHost = rawHost;
+   this.reverseHost = ReverseHostName( host );
    this.path = path;
    this.isSecure = isSecure;
    this.expires = expires;
***************
*** 146,152 ****
    cookiesTree.treeBoxObject.view = cookiesTreeView;
  
   // sort by host column
!   CookieColumnSort('rawHost');
  
    // disable "remove all cookies" button if there are no cookies
    document.getElementById("removeAllCookies").disabled = cookies.length == 0;
--- 147,153 ----
    cookiesTree.treeBoxObject.view = cookiesTreeView;
  
   // sort by host column
!   CookieColumnSort('reverseHost');
  
    // disable "remove all cookies" button if there are no cookies
    document.getElementById("removeAllCookies").disabled = cookies.length == 0;
diff -r -c ./mozilla/browser/components/cookieviewer/content/CookieViewer.xul
./mozilla.new/browser/components/cookieviewer/content/CookieViewer.xul
*** ./mozilla/browser/components/cookieviewer/content/CookieViewer.xul	Sun Apr
25 14:11:57 2004
--- ./mozilla.new/browser/components/cookieviewer/content/CookieViewer.xul	Mon
Sep 27 18:59:06 2004
***************
*** 49,55 ****
                  hidecolumnpicker="true">
          <treecols>
            <treecol id="domainCol" label="&treehead.cookiedomain.label;" flex="2"
!                        onclick="CookieColumnSort('rawHost', true);"
persist="width"/>
            <splitter class="tree-splitter"/>
            <treecol id="nameCol" label="&treehead.cookiename.label;" flex="1"
                         onclick="CookieColumnSort('name', true);" persist="width"/>
--- 49,55 ----
                  hidecolumnpicker="true">
          <treecols>
            <treecol id="domainCol" label="&treehead.cookiedomain.label;" flex="2"
!                        onclick="CookieColumnSort('reverseHost', true);"
persist="width"/>
            <splitter class="tree-splitter"/>
            <treecol id="nameCol" label="&treehead.cookiename.label;" flex="1"
                         onclick="CookieColumnSort('name', true);" persist="width"/>
diff -r -c ./mozilla/browser/components/cookieviewer/content/treeUtils.js
./mozilla.new/browser/components/cookieviewer/content/treeUtils.js
*** ./mozilla/browser/components/cookieviewer/content/treeUtils.js	Tue Feb 24
20:52:16 2004
--- ./mozilla.new/browser/components/cookieviewer/content/treeUtils.js	Mon Sep
27 18:58:58 2004
***************
*** 144,146 ****
--- 144,161 ----
    return ascending;
  }
  
+ function ReverseHostName( hostname ) {
+   // This function will take a host name like bugzilla.mozilla.org and
+   // reformat it to org.mozilla.bugzilla.  This way when the sorting is done
+   // hosts from the same domain will be next to each other.
+   if (hostname == "") {
+     return hostname;
+   }
+   // Delete any ':' or ' ' (space) and anything that follows it.  This is due
+   // to the fact that sometimes they store the port number with the host name
+   // in the cookie.  I think they are going to stop doing that but at the
+   // moment they still are.
+   var workname = hostname.replace(/[:\s].*/, "");
+   // Reverse the name and return it
+   return workname.split(".").reverse().join(".");
+ }
diff -r -c ./mozilla/browser/components/prefwindow/content/permissions.js
./mozilla.new/browser/components/prefwindow/content/permissions.js
*** ./mozilla/browser/components/prefwindow/content/permissions.js	Sat Aug 23
19:01:13 2003
--- ./mozilla.new/browser/components/prefwindow/content/permissions.js	Tue Sep
28 11:09:38 2004
***************
*** 82,87 ****
--- 82,88 ----
    this.number = number;
    this.host = host;
    this.rawHost = rawHost;
+   this.reverseHost = ReverseHostName( host );
    this.type = type;
    this.capability = capability;
    this.perm = perm;
***************
*** 158,164 ****
  
    // sort and display the table
    permissionsTree.treeBoxObject.view = permissionsTreeView;
!   PermissionColumnSort('rawHost', false);
  
    // disable "remove all" button if there are none
    document.getElementById("removeAllPermissions").disabled =
permissions.length == 0;
--- 159,165 ----
  
    // sort and display the table
    permissionsTree.treeBoxObject.view = permissionsTreeView;
!   PermissionColumnSort('reverseHost', false);
  
    // disable "remove all" button if there are none
    document.getElementById("removeAllPermissions").disabled =
permissions.length == 0;

Comment 37

[benc]

If possible, I'd like to have one sorting function that solves the DNS + IPv4
problem in a central location. While hacking every module locally sounds not to
time-consuming, don't forget we eventually will have to re-hack for IPv6.

I've created bug 276474 for such a purpose. I've also got about 50% of the
javascript needed for such a function working. I've moved the bugs that depend
on this bug to the new bug.

Comment 38

[Jo Hermans]

*** Bug 291184 has been marked as a duplicate of this bug. ***

Comment 39

[Jo Hermans]

*** Bug 295952 has been marked as a duplicate of this bug. ***

Comment 40

[Mike Connor [:mconnor]]

This isn't the right bug, and would have a vastly different fix.

Comment 41

[Mike Connor [:mconnor]]

Not going to be working on any Seamonkey UI bugs for the foreseeable future. 
You can filter on "danlikesgoats" to delete this spam.

Comment 46

[Stefan]

Seriously, this has been reported in 2002, there is a patch available since 2004, the list of duplicates keeps adding up, and no-one has taken the time to fix this ever since?

Comment 47

[Shawn Wilsher :sdwilsh]

The patch does not apply to any code that has been around recently.

Comment 48

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8 (if account gets disabled due to email bounces, ask a bugzilla admin to reenable it)]

Created attachment 460158 [details] [diff] [review]
patch for Firefox cookie *exceptions* window

I've had this in my tree for ages; I no longer remember what parts of it I wrote (though I'm pretty sure I took at least parts from something else in this bug).

This just fixes the cookie exceptions window and not the cookie manager window.

Comment 49

[Shawn Wilsher :sdwilsh]

Comment on attachment 460158 [details] [diff] [review]
patch for Firefox cookie *exceptions* window

I think dwitte can review this...

Comment 50

[[e-mail]]

Comment on attachment 460158 [details] [diff] [review]
patch for Firefox cookie *exceptions* window

Sure. A screenshot would help, so I can make sure it looks sensible, but I trust that it does.

This won't exactly work for IP addresses, but there's no harm in grouping them strangely in the sort order, I suppose.

r=dwitte

Comment 51

[Marcelo Bastos]

If the new "Data Manager" Kairo introduceded in Seamonkey lands on Firefox (I understand it's being ported), wouldn't this bug be considered fixed?

Comment 52

[[Baboo]]

It's called the permissions manager in Firefox and is accessible in the current nightlies through about:permissions

And currently the cookie manager is not obsoleted, in fact if you click "Manage Cookies…" for a domain in the permissions manager it simply opens the cookie manager with the search box filled out.

Comment 53

[Alain Knaff]

(In reply to [Baboo] from comment #52)
> It's called the permissions manager in Firefox and is accessible in the
> current nightlies through about:permissions

Thanks, you made my day. I've been looking for this feature for months!

> 
> And currently the cookie manager is not obsoleted, in fact if you click
> "Manage Cookies…" for a domain in the permissions manager it simply opens
> the cookie manager with the search box filled out.

Comment 56

[jbrown]

With the new tab-based preferences it's even worse since it takes into account the http/https prefix now. If there is a website with both http and https cookie exceptions you don't find them near to each other, but you'll see

http://bar.foo
http://foo.bar
https:/bar.foo
https:/foo.bar

So either put http/https into a separate column to allow website ordering or disregard the http/https prefix when doing the sorting, so that we get

http://bar.foo
https:/bar.foo
http://foo.bar
https:/foo.bar

Comment 57

[jbrown]

Why is there a differentiation between http and https after all?

Comment 58

[:Gijs (he/him)]

Good news, this works in the new data manager!