Closed Bug 1251368 Opened 4 years ago Closed 4 years ago

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

Categories

(SeaMonkey :: Passwords & Permissions, defect)

SeaMonkey 2.44 Branch
defect
Not set

Tracking

(seamonkey2.44+ fixed, seamonkey2.46+ fixed)

RESOLVED FIXED
seamonkey2.45
Tracking Status
seamonkey2.44 + fixed
seamonkey2.46 + fixed

People

(Reporter: philip.chee, Assigned: frg)

References

Details

Attachments

(2 files, 1 obsolete file)

No description provided.
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.
Version: SeaMonkey 2.42 Branch → SeaMonkey 2.44 Branch
2.43 is not affected.
Attached patch 1251368-cookieremove.patch (obsolete) — Splinter Review
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.
Attachment #8729829 - Flags: review?(philip.chee)
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Depends on: 1188348
Duplicate of this bug: 1260120
Depends on: 1245184
Component: Preferences → Passwords & Permissions
As mentioned by aceman on IRC for TB, is a port for bug 1199466 needed too?
Flags: needinfo?(frgrahl)
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.
Flags: needinfo?(frgrahl)
See Also: → 1256153
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.
Attachment #8729829 - Flags: review?(philip.chee) → review+
Update patch with slice as per review comment.

Review+ from Philip Chee carried forward.
Attachment #8729829 - Attachment is obsolete: true
Attachment #8737552 - Flags: review+
Keywords: checkin-needed
http://hg.mozilla.org/comm-central/rev/36733e9c1206
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.45
Blocks: 1263252
You need to log in before you can comment on or make changes to this bug.