Closed Bug 142179 Opened 23 years ago Closed 7 years ago

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

Categories

(Firefox :: Settings UI, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: John.L.Villalovos, Unassigned)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

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.
Attached patch An attempt to fix the problem (obsolete) — Splinter Review
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
Attached patch Second attempt to fix problem (obsolete) — Splinter Review
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.
*** Bug 131503 has been marked as a duplicate of this bug. ***
Updating platform, OS, and severity. Would like to see this functionality.
Severity: minor → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
Filed bug 143166 which requests this behavior for image blocking sites as well.
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.
Blocks: 107957
Blocks: 143472
No longer blocks: 107957
Blocks: 143166
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.
Attached patch Fix part 3 (obsolete) — Splinter Review
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.
Target Milestone: --- → mozilla1.0.1
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0.1 → mozilla1.1alpha
Priority: -- → P3
Target Milestone: mozilla1.1alpha → mozilla1.2alpha
Aren't ports supposed to be stripped off and not stored? I'm thinking of bug 142803 here.
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(".");
Attachment #83080 - Flags: needs-work+
Attached patch Patch to fix this bug. Works :) (obsolete) — Splinter Review
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.
Attachment #83080 - Attachment is obsolete: true
I had left a .orig file lying around in the previous patch :(
Attachment #91675 - Attachment is obsolete: true
*** Bug 160632 has been marked as a duplicate of this bug. ***
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?
Attachment #91677 - Flags: review+
Attachment #82367 - Attachment is obsolete: true
Attachment #82369 - Attachment is obsolete: true
I made previous patches obsolete
Target Milestone: mozilla1.2alpha → mozilla1.3alpha
-> suresh
Assignee: morse → suresh
Status: ASSIGNED → NEW
Blocks: 194878
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).
QA Contact: tever → cookieqa
Summary: Sort order in "Cookie Sites" needs to be done by Top Level Domain on down → Cookie Manager: sort "Cookie Sites" should sort by Domain (TLD -> left, as labels, not strings, etc.)
*** Bug 202044 has been marked as a duplicate of this bug. ***
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.
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.
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.
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.
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?
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.
> 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.
Unfortunately the patch no longer works; furthermore it doesn't deal with IP addressed cookies as was pointed out in comment 24.
> 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 :-).
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.
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.
-> 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.
Assignee: skasinathan → mconnor
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.
*** Bug 240028 has been marked as a duplicate of this bug. ***
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.
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?
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.
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;
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.
No longer blocks: 143166, 143472, 194878
Depends on: 276474
Blocks: 222559
*** Bug 291184 has been marked as a duplicate of this bug. ***
*** Bug 295952 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.1?
This isn't the right bug, and would have a vastly different fix.
Flags: blocking-aviary1.1? → blocking-aviary1.1-
Not going to be working on any Seamonkey UI bugs for the foreseeable future. You can filter on "danlikesgoats" to delete this spam.
Assignee: mconnor → nobody
Priority: P3 → --
Target Milestone: mozilla1.3alpha → ---
Depends on: 331510
Blocks: 100573
Depends on: 282220
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?
The patch does not apply to any code that has been around recently.
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 on attachment 460158 [details] [diff] [review] patch for Firefox cookie *exceptions* window I think dwitte can review this...
Attachment #460158 - Flags: review?(dwitte)
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
Attachment #460158 - Flags: review?(dwitte) → review+
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?
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.
(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.
Component: Networking: Cookies → Preferences
Product: Core → Firefox
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
Why is there a differentiation between http and https after all?
Good news, this works in the new data manager!
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: