238931 - remove the magic number 8 (kill ALLOW_SESSION_ONLY const)

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect, P1)

Description

[Mike Connor [:mconnor]]

we should get rid of the magical number 8 for session perms before 1.7 as both
Camino and Firefox are planning to branch from 1.7 after it goes final, and I'd
rather not make them perpetuate ugly hacks on their 1.0 branches.

http://bugzilla.mozilla.org/show_bug.cgi?id=225857#c7 indicated dwitte's okay
with it going into nsICookiePermission.idl, which is good enough for me.

Comment 1

[Mike Connor [:mconnor]]

Attachment: dwitte's dirty work done

Comment 2

[[e-mail]]

Comment on attachment 144933 [details] [diff] [review]
dwitte's dirty work done

ah yes, this dirty laundry has been lying in the corner for quite some time
now...

>Index: mozilla/netwerk/cookie/public/nsICookiePermission.idl
>===================================================================
>   /**
>    * nsCookieAccess values
>+   * 
>+   * ACCESS_SESSION doesn't really belong here, the cookieservice doesn't
>+   * need to know about internal constants used by the manager, but this 
>+   * is a better place than anywhere else.  If this affects freezing this 
>+   * interface we may relocate it elsewhere	
>    */
>   const nsCookieAccess ACCESS_DEFAULT = 0;
>   const nsCookieAccess ACCESS_ALLOW   = 1;
>   const nsCookieAccess ACCESS_DENY    = 2;
>+  const nsCookieAccess ACCESS_SESSION = 8;

i'd like to see it made more explicit here, that ACCESS_SESSION is not an
allowed return value of nsICookiePermission:canAccess.

so, maybe separate out ACCESS_SESSION with its own little comment like so:

  /**
   * additional values for nsCookieAccess, which are not directly used by
   * any methods on this interface, but are nevertheless convenient to define
   * here. these may be relocated somewhere else if we ever consider freezing
   * this interface.
   */
  const nsCookieAccess ACCESS_SESSION = 8;

and then in the comment for canAccess, do something like so:

   * @return one of the following nsCookieAccess values:
   *	     ACCESS_DEFAULT, ACCESS_ALLOW, or ACCESS_DENY

with those changes, r=me, thanks! (btw, painful i know, but you might want to
grep around camino/ff/etc and change those ones too; i know camino has
instances of 8...)

Comment 3

[Mike Connor [:mconnor]]

Comment on attachment 144933 [details] [diff] [review]
dwitte's dirty work done

well, it took a few months, but that ugly const is getting killed.

alec, can you SR this assuming that I'll adopt dwitte's comments before
checkin?  This patch is in a tree on the other half of my dual-boot

Comment 4

[Mike Connor [:mconnor]]

Attachment: dwitte's dirty work done, with comments from dwitte!

Comment 5

[Mike Connor [:mconnor]]

Comment on attachment 145237 [details] [diff] [review]
dwitte's dirty work done, with comments from dwitte!

alec, I'm hoping to get this in before 1.7 so when Firefox and Camino branch
for 1.0, we don't make them carry along this baggage.  If you can't get to this
soon, let me know and I'll find someone else.

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 145237 [details] [diff] [review]
dwitte's dirty work done, with comments from dwitte!

darin, no reply from alecf, this is pretty straightforward, and I think if 1.7
turns into the stable branch we really want to have this there.

Comment 7

[Asa Dotzler [:asa]]

Comment on attachment 145237 [details] [diff] [review]
dwitte's dirty work done, with comments from dwitte!

a=asa (on behalf of drivers) for checkin to 1.7

Comment 8

[Mike Connor [:mconnor]]

checked in 2004-04-07 16:27