User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36 Steps to reproduce: Using chrome.cookies.set to set a session cookie. 'details' parameter contains url, name, value, httpOnly=true url is an HTTP url with port number 'callback' parameter specifies the callback function The following page says that setting session cookies might fail: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Chrome_incompatibilities See cookies section. However, there was no corresponding bug to fix this. Actual results: In the callback function, 'cookie' parameter is null. chrome.extension.lastError is undefined (this is another bug.) Session cookie is not set. Expected results: Session cookie should be set. 'cookie' parameter should contain the set cookie data.
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
Priority: -- → P2
Whiteboard: [cookies] triaged
Can you please provide a testcase extension that shows the problem?
Test case for session cookie set failure
It looks like there is a workaround. If I set expirationDate then the session cookie is set.
Hm. Yeah. Apparently we set an expiration time of 0 for session cookies, and the cookie service honors that. The cookie service itself uses INT64_MAX. We also apparently need tests for session cookies.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-webextensions? → blocking-webextensions+
Whiteboard: [cookies] triaged → [cookies][good first bug] triaged
(In reply to Michail Frolov from comment #3) > It looks like there is a workaround. > > If I set expirationDate then the session cookie is set. I guess this makes the cookie permanent instead of session. Not really a workaround.
Attachment #8715097 - Attachment mime type: application/zip → application/x-xpinstall
Review commit: https://reviewboard.mozilla.org/r/35887/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/35887/
Attachment #8722162 - Flags: review?(kmaglione+bmo)
Comment on attachment 8722162 [details] MozReview Request: Bug 1238269 - Setting session cookie using chrome.cookies.set fails, r?kmag Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35887/diff/1-2/
Note that in addition to fixing the bug and adding tests for session cookies, I also cleaned up the existing tests. Some of the assert messages were inconsistent in their statement, the order of the arguments for a number of the the `assertEq` were incorrect, and the expected order of arguments for the internal `assertExpected` was counter-intuitive as it does not match the standard `assertEq`. I apologize that this will add a bit of extra time to the review, but I think it's worth the effort to have tests that are consistent and accurate in their messages and use of the `assertEq` API.
Comment on attachment 8722162 [details] MozReview Request: Bug 1238269 - Setting session cookie using chrome.cookies.set fails, r?kmag https://reviewboard.mozilla.org/r/35887/#review32543 ::: toolkit/components/extensions/test/mochitest/test_ext_cookies.html:64 (Diff revision 2) > - browser.test.assertEq(cookie.hostOnly, false, "not a hostOnly cookie"); > + browser.test.assertEq(false, cookie.hostOnly, "cookie is hostOnly cookie"); *is not a host only cookie ::: toolkit/components/extensions/test/mochitest/test_ext_cookies.html:68 (Diff revision 2) > + // create a session cookie Please capitalize and end with a period. ::: toolkit/components/extensions/test/mochitest/test_ext_cookies.html:74 (Diff revision 2) > + browser.test.assertEq(true, cookie.session, "got session cookie"); We should remove this cookie, too, and ideally test the removal. ::: toolkit/components/extensions/test/mochitest/test_ext_cookies.html:76 (Diff revision 2) > - browser.test.notifyPass("cookies"); > + browser.test.notifyPass("cookies"); This doesn't need its own promise handler. It can go in the last one.
Attachment #8722162 - Flags: review?(kmaglione+bmo) → review+
Comment on attachment 8722162 [details] MozReview Request: Bug 1238269 - Setting session cookie using chrome.cookies.set fails, r?kmag Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35887/diff/2-3/
https://reviewboard.mozilla.org/r/35887/#review32543 > *is not a host only cookie Oops. Fixed.
Is there any proper way of manually testing this bug? If yes, could you please provide some reliable steps?
There was a test case extension attached in comment 2, so you could load that into your instance of Firefox and then check the console log for messages. It looks like with the bug the log will show the cookie as `null`. I assume that with the bug fixed you would see some output other than `null`.
(In reply to Bob Silverberg [:bsilverberg] from comment #16) > There was a test case extension attached in comment 2, so you could load > that into your instance of Firefox and then check the console log for > messages. It looks like with the bug the log will show the cookie as `null`. > I assume that with the bug fixed you would see some output other than `null`. Thanks Bob! You were right: http://i.imgur.com/mrcLKef.jpg I was able to reproduce the initial issue on Firefox 46.0a1 (2016-01-25) under Windows 10 64-bit. Verified fixed on Firefox 48.0a1 (2016-04-04) and Firefox 46.0a2 (2016-04-04) under Windows 10 64-bit, Mac OS X 10.11 and Ubuntu 12.04 32-bit.
You need to log in before you can comment on or make changes to this bug.