1251368 - SeaMonkey cookie code needs to be updated to take into account mozilla-central Bug 1245184

Status: RESOLVED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Comment 1

[Frank-Rainer Grahl (:frg)]

Attachment: cookies.sqlite originAttributes

The good thing is that this should be easy to implement. I turned on cookies and browsed for a while with 2.45a1. The originAttributes for all created cookies was always blank. When removing a cookie I got a 

>> Timestamp: 3/8/2016 7:36:22 PM
>> Error: NS_ERROR_XPC_NOT_ENOUGH_ARGS: Not enough arguments 
>> [nsICookieManager.remove]
>> Source File: chrome://communicator/content/permissions/cookieViewer.js
>> Line: 291

in the old cookieviewer and

>> Timestamp: 3/8/2016 7:38:24 PM
>> Error: NS_ERROR_XPC_NOT_ENOUGH_ARGS: Not enough arguments 
>> [nsICookieManager2.remove]
>> Source File: chrome://communicator/content/dataman/dataman.js
>> Line: 1118

in Data Manager. Can look at it when Bug 1188348 is checked in (hint hint hint...). Dont want to mess further with the components before. Jiggling around the patches would get messy. Too many files affected at once.

Comment 2

[Frank-Rainer Grahl (:frg)]

2.43 is not affected.

Comment 3

[Frank-Rainer Grahl (:frg)]

Attachment: 1251368-cookieremove.patch

Fix for Data Manager and Cookie Viewer. I only found these 2 places in suite where this was used. The patch goes on top of the patch for Bug 1188348. Tested only on c-a due to current c-c breakage but this shouldn't matter. 

mail and chat are also affected.

suite tests not fixed.

Comment 5

[Ian Neal]

As mentioned by aceman on IRC for TB, is a port for bug 1199466 needed too?

Comment 6

[Frank-Rainer Grahl (:frg)]

Ian,

I would say no. The attribute is always blank. See the jpeg above. I do not think this is useful for Seamonkey right now. See comment 3 in bug 1199466:

>> For now, we only have 4 user contexts.  The user doesn't know they correspond to numbers.  
>> So I would say show the user context name instead of the number (Personal, Work, Banking, 
>> Shopping) under a column named "Container".

I thought it was only used for mobile but it seems to be a new feature which we do not use yet. Maybe should be ported in a new bug but briefly looking at I I do not find it very useful.

Comment 7

[Philip Chee]

Comment on attachment 8729829 [details] [diff] [review]
1251368-cookieremove.patch

> -function Cookie(id,name,value,isDomain,host,rawHost,path,isSecure,expires) {
> +function Cookie(id,name,value,isDomain,host,rawHost,path,originAttributes,isSecure,expires) {

Nit:
function Cookie(id, name, value, isDomain, host, rawHost, path,
                originAttributes, isSecure, expires) {

>        new Cookie(count++, nextCookie.name, nextCookie.value, nextCookie.isDomain, host,
>                   (host.charAt(0)==".") ? host.substring(1,host.length) : host,
> -                 nextCookie.path, nextCookie.isSecure, nextCookie.expires);
> +                 nextCookie.path, nextCookie.originAttributes, nextCookie.isSecure, nextCookie.expires);

Nit:
      new Cookie(count++, nextCookie.name, nextCookie.value,
                 nextCookie.isDomain, host,
                 host.charAt(0) == "." ? host.substring(1, host.length) : host,
                 nextCookie.path, nextCookie.originAttributes,
                 nextCookie.isSecure, nextCookie.expires);

Suggestion:
host.substring(1)
or
host.slice(1)

r=me with the above fixed.

Comment 8

[Frank-Rainer Grahl (:frg)]

Attachment: 1251368-cookieremove-V1a.patch

Update patch with slice as per review comment.

Review+ from Philip Chee carried forward.

Comment 9

[Philip Chee]

http://hg.mozilla.org/comm-central/rev/36733e9c1206

Comment 10

[Philip Chee]

http://hg.mozilla.org/releases/comm-aurora/rev/dd86a1b531ee
http://hg.mozilla.org/releases/comm-aurora/rev/7d7b42759895