220067 - Cookie Manager: sort by Cookie Name should be case-insensitive

Status: RESOLVED FIXED

(Core :: Networking: Cookies, enhancement)

Description

[Marc Boon]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20030916
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20030916

Or better still, use case-insensitive natural-order sorting.
It's a matter of taste I guess...


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Emmanuel Bourg]

Attachment: Case insensitive sort for the cookie manager

That's not the most optimized solution but it works.

Comment 2

[Darin Fisher]

Comment on attachment 131977 [details] [diff] [review]
Case insensitive sort for the cookie manager

hmm... i wonder how this will work with non-ASCII text.  maybe cookie names are
constrained to ASCII?

Comment 3

[Emmanuel Bourg]

I'm not familiar with the code of the cookie manager but it seems it only deals
with strings for sorting operations.

Comment 4

[Emmanuel Bourg]

http://www.ietf.org/rfc/rfc2965.txt
"   NAME=VALUE
      REQUIRED.  The name of the state information ("cookie") is NAME,
      and its value is VALUE.  NAMEs that begin with $ are reserved and
      MUST NOT be used by applications.

      The VALUE is opaque to the user agent and may be anything the
      origin server chooses to send, possibly in a server-selected
      printable ASCII encoding.  "Opaque" implies that the content is of
      interest and relevance only to the origin server.  The content
      may, in fact, be readable by anyone that examines the Set-Cookie2
      header."

I'm not sure that means the cookie name is in ASCII as well. Cookie names in
Java are restricted to ASCII only :
http://java.sun.com/j2ee/sdk_1.3/techdocs/api/javax/servlet/http/Cookie.html
"The name must conform to RFC 2109. That means it can contain only ASCII
alphanumeric characters and cannot contain commas, semicolons, or white space or
begin with a $ character"

Comment 5

[Emmanuel Bourg]

Attachment: Case insensitive sort for the cookie manager

This version is more careful about the type of the elements sorted. It also
removes the duplication of the comparator.

Comment 6

[[e-mail]]

Comment on attachment 132082 [details] [diff] [review]
Case insensitive sort for the cookie manager

sure, why not.

r=me

Comment 7

[Darin Fisher]

Comment on attachment 132082 [details] [diff] [review]
Case insensitive sort for the cookie manager

>Index: extensions/wallet/cookieviewer/nsWalletTreeUtils.js

>+      return -CompareLowerCase(first[column], second[column]);

nit:	return CompareLowerCase(second[column], first[column]);


>+/**
>+ * Compare two variables. The comparison is non case sensitive if the specified
>+ * parameters are strings.
>+ */
>+function CompareLowerCase(param1, param2) {
>+
>+  var p1 = typeof(param1) == "string" ? param1.toLowerCase() : param1;
>+  var p2 = typeof(param2) == "string" ? param2.toLowerCase() : param2;

why is this type checking necessary?  are there really cases where the
parameters would not be ASCII strings?

Comment 8

[Emmanuel Bourg]

There are two versions of nsWalletTreeUtils.js in CVS located at
mozilla/browser/components/cookieviewer/content and
mozilla/extensions/wallet/cookieviewer. Which one should be modified ?

Comment 9

[[e-mail]]

erm, did bryner just copy that file into firebird? sigh.

modify both, please.

Comment 10

[[e-mail]]

actually, just provide a patch for the one in extensions/cookieviewer, and i'll
cc bryner. he can apply the patch to firebird if he wants to or not.

Comment 11

[Emmanuel Bourg]

Attachment: Case insensitive sort for the cookie manager

Updated patch integrating Darin's suggestions.

Comment 12

[Emmanuel Bourg]

I think i'll put back the type checking in the patch, it's needed to sort
cookies by expiration time in Bug 162914

Comment 13

[Darin Fisher]

Comment on attachment 132410 [details] [diff] [review]
Case insensitive sort for the cookie manager

sr=darin

dwitte: anyone want to volunteer to check this in?

Comment 14

[[e-mail]]

checked in... thanks for the patch.

bryner: would you like to apply this patch to firebird, also?