Open Bug 282220 Opened 20 years ago Updated 2 years ago

Sort domain lists (cookies, exceptions, saved passwords, etc) in reverse DNS order (optionally?)

Categories

(Firefox :: Settings UI, enhancement)

enhancement

Tracking

()

People

(Reporter: msherman, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

Any list of domain/url based items in the preferences should be (optionally?)
sorted in reverse-domain order.  That way, for example, a cookie exception for
secure.mozilla.org would sort adjacent to www.mozilla.org.

Currently, it's very difficult to find a cookie exception in the list, because
it's very large, and sites often save multiple cookies under many different
sub-domain names.  I've had to resort to shutting down firefox and searching the
prefs file with a text editor in the past.

Reproducible: Always
The code is not in toolkit, ->Firefox
Product: Toolkit → Firefox
QA Contact: nobody → preferences
Please consider right-justifying the resulting list.  This makes it visually
easier to pick out the domain names.
*** Bug 346127 has been marked as a duplicate of this bug. ***
This seems to be a duplicate of bug 269696, though the summary has a broader scope.
Depends on: 276474
Blocks: 376682
Hardware: PC → All
Attached patch patch (obsolete) — Splinter Review
A while back I patched content/browser/preferences/permissionsutils.js in chrome/browser.jar, to sort the cookies exception list in reverse DNS order, but didn't bother to take exception for all numeric address/IP. I've just recently added code to take care of that.

Every time firefox updates I have to go in and patch the jar file again (not too hard with xemacs).  It would be nice if something like this would make it's way into a future version.  I believe the same code is used to sort other permissions, thus the name of the file.

I'll attach the current version of the patch, which I've only tested on Linux/Fedora 8/Firefox 2.0.0.10-3.
Attached patch cvs patch (obsolete) — Splinter Review
Replacing previous patch, generated with patch, with cvs diff -up9w patch
Attachment #293464 - Attachment is obsolete: true
Attachment #293470 - Flags: review?(mano)
Comment on attachment 293470 [details] [diff] [review]
cvs patch

Some comments would be nice.

>Index: browser/components/preferences/permissionsutils.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/preferences/permissionsutils.js,v
>retrieving revision 1.5
>diff -u -p -8 -w -r1.5 permissionsutils.js
>--- browser/components/preferences/permissionsutils.js	5 Apr 2007 09:59:20 -0000	1.5
>+++ browser/components/preferences/permissionsutils.js	17 Dec 2007 07:19:27 -0000
>@@ -82,21 +82,42 @@ var gTreeUtils = {
>     }
>     selection.selectEventsSuppressed = false;
>   },
>   
>   sort: function (aTree, aView, aDataSet, aColumn, 
>                   aLastSortColumn, aLastSortAscending) 
>   {
>     var ascending = (aColumn == aLastSortColumn) ? !aLastSortAscending : true;
>-    aDataSet.sort(function (a, b) { return a[aColumn].toLowerCase().localeCompare(b[aColumn].toLowerCase()); });
>+    var numeric = /^(?:\d{1,3}\.){3}\d{1,3}$/;
>+    aDataSet.sort
>+    (function (a, b)
>+     {
>+       var aNumeric = numeric.test (a[aColumn]);
>+       var bNumeric = numeric.test (b[aColumn]);
>+       if (!aNumeric && !bNumeric) {
>+         return a[aColumn].toLowerCase().split('.').reverse().join('.').localeCompare(b[aColumn].toLowerCase().split('.').reverse().join('.'));

Please add some variables here to make this a little bit more self-explaining

>+       } else if (aNumeric && bNumeric) {

No |else| after |return|.

>+         var aComponents = a[aColumn].split('.');
>+         var bComponents = b[aColumn].split('.');
>+         for (i = 0; i < 4; i++) {
>+           if (aComponents[i] != bComponents[i]) {
>+             return aComponents[i] - bComponents[i];

I don't understand this code path altogether.

>+           }
>+         }
>+         return 0;
>+       } else if (aNumeric && !bNumeric) {

ditto.

>+         return -1;
>+       }

>+       return 1;

How would you get here?
Attachment #293470 - Flags: review?(mano) → review-
> Some comments would be nice.

Just conforming to the original style of the file, which doesn't have a single comment in it :-)

I'll add an updated patch with comments that I think answer Asaf's questions.
commented code, variables added/renamed
Attachment #293470 - Attachment is obsolete: true
Attachment #298052 - Flags: review?(mano)
Version: unspecified → Trunk
What's the status of this thing?
Comment on attachment 298052 [details] [diff] [review]
replaces sort routine in permissionutils.js

Looks good to me at first glance. My only concern is that this code should be easy to re-use where we need it. Perhaps we should have it live in a JavaScript module in toolkit so that it can be used for bug 269696 as well? Would also be good to have a test added for it, as mentioned in bug 269696 comment 11. Dolske or I can help provide guidance on testing or implementing the module if needed.
Attachment #298052 - Flags: review?(mano)
And apologies for the long delay in response - you should feel free to poke people directly and be more forceful if you don't get a response in a reasonable amount of time!
(In reply to comment #13)
> *** Bug 569231 has been marked as a duplicate of this bug. ***

... and I searched :-) ...
Just apply the patch above to content/browser/preferences/permissionsutils.js in chrome/browser.jar and you'll be on your way. And you'll need to keep patching any time firefox is updated.

I've been doing this for several years now and it still works today with firefox 3.6.3. My patch was rejected for lack of comments, when the original file had none, and I just don't have the time to pursue this through any further.  I started adding code for ipv6 addresses but haven't finished, and probably won't until my cookie list contains several raw ipv6 addresses in it.
> Just apply the patch above to content/browser/preferences/permissionsutils.js
in chrome/browser.jar and you'll be on your way. And you'll need to keep
patching any time firefox is updated.

Do you know of any tools for patching jar files?

Or is it
unzip to a temp directory, abort if error
patch in that temp directory, abort if error
rebuild zip from temp, abort if error
Safely replace old file with new, and undo if error

(Note that step 4 might be hard to do in a cross-system environment :-)
I've done that in the past (with jar, now gjar on linux) but now I just edit the whole thing directly with xemacs, which takes care of all those details for me.
this would fix bug 142179
Blocks: 142179
The title of this bug could usefully mention "history" (as in the browser history list) before "etc", for the benefit of people searching Bugzilla.
This bugzilla entry is more than 5 years old (and my patch above fixes it) that more appropriate would be to mention "ancient history" in the title
(In reply to comment #16)
> > Just apply the patch above to content/browser/preferences/permissionsutils.js
> in chrome/browser.jar and you'll be on your way. And you'll need to keep
> patching any time firefox is updated.
> 
> Do you know of any tools for patching jar files?
> 
> Or is it
> unzip to a temp directory, abort if error
> patch in that temp directory, abort if error
> rebuild zip from temp, abort if error
> Safely replace old file with new, and undo if error
> 
> (Note that step 4 might be hard to do in a cross-system environment :-)

Ok, I just did all this on 3.6.17
Kleiman-ibook:preferences michael$ pbpaste | patch
patching file permissionsutils.js
Hunk #1 succeeded at 45 (offset -36 lines).
Kleiman-ibook:preferences michael$ 

But the sort order has not changed.
After rebuilding the .jar file (a renamed .zip archive, right?), and restarting firefox, I tested by going to the saved passwords in Firefox preferences -- still sorting by alphabetical, not reverse dns.

(And yes, addons like Cookie Culler are also displaying in alphabetical, not reserve dns).

Has the latest firefox broken this? This is the first time I tried applying this patch.
Still works for me on 3.6.17.

But meanwhile I found a bug in my patch where google and googlelabs (and similar constructs) can be mixed in the output.

Fixed it, will post it here later, or as part of fixing <a href="show_bug.cgi?id=269696" title="Password list is in alpha order, not sorted by domain name">bug 269696</a>.
@Jack,
Would you please evaluate this bug to see if this helps about:preferences UX ?
Thank you
Flags: needinfo?(jalin)
In my opinion, 
1. One question in my mind is - When users remove a cookie, would they like to remove other relative cookies at the same time? If not, should we have sorting by DNS?
2. Usually there are hundreds of items on the list, so even the list is sorted by DNS, I guess it still looks messy?! Moreover, if we provide sorting by DNS, I think the next issue would be - How do users easily find a specific domain on the list which is sorting by DNS?
3. For now, using "Search" to find an item is an easy and quick way.

So, I would prefer not to do this feature.
Flags: needinfo?(jalin)
Hi, Jack.

1. Yes, multiple entries. When I go to remove a cookie, it's usually either because a (buggy) web site isn't letting me log in when it should, or something like that; or because I don't want the (untrusted) web site to remember me for security or privacy reasons. In either case, I don't know exactly which cookie I'm concerned about, and want to remove all cookies associated with the site and all its associated domains. When I go to remove a saved password, likewise I typically want to review and remove all entries related to that organization. When I go to search my History, I typically want to see all my visits to Google, or the BBC, grouped together, not one specific address.

2. How do users find a specific domain? Same as when you look for someone's name on a list of (Firstname, Lastname) sorted by Lastname -- you scroll up and down looking for the Lastname and then look for the Firstname. Looking for bugzilla.mozilla.org? You quickly notice that names beginning with 'bugzilla' aren't grouped near the top of the list, rather names ending with '.au', '.be', '.ca', then '.com', then '.org', so scroll down to there and then down to where they start ending with '.mozilla.org'. (Or use the search bar, of course.)

3. Search works well for many cases, but with important exceptions. For example searching cookies for 'google' throws up so many non-Google cookies that contain 'google' in their content, in among so many different Google domains, that it's hard to quickly select only the google-domain ones.

BTW, sorting Firefox's page History list by the Location column sorts all 'http' addresses before all 'https' addresses, which is an even worse example of the same basic problem.

(I'm a FF user and other software dev, not a FF dev.)
Reply to Jack Lin from a regular user:

1. When I remove a cookie, I almost always want to remove _all_ cookies from the same site (domain) -- or, at least, I want to look through all such cookies and choose the ones I want to remove (so, same benefit to keeping them all toegther). Whether sorting should be, e.g., org->mozilla->bugzilla->https or mozilla.org->bugzilla->https doesn't matter to me: either way would be vastly superior to the current behavior. (Second way is probably preferable for most users.)

2. I don't really understand your question, but if a user is looking to remove all cookies from, say, all Wikipedia(s), they will be looking for "wikipedia.org", not "en.wikipedia.org", "de.wikipedia.org", etc., mixed in with "discover.com", "ebay.com", "en.wiktionary", etc. Even if you don't remember Wikipedia is a .org site, finding it (/them all) in a list in org->wikipedia->en order would be much easier than it is currently.

3. "Search" is only available for removing cookies and (as a blunt instrument) viewing individual history items, not cookie _exceptions_ nor viewing the history _hierarchy_.

Variations on this feature request have come up so often, it's clear that the current way is not desireable for many users. (OTOH, making it optional is preferable, in case some people would prefer the current behavior.)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: