Add support for getting and removing cookies from the page

RESOLVED FIXED in Firefox 18

Status

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: automatedtester, Assigned: automatedtester)

Tracking

(Blocks 1 bug)

unspecified
mozilla20
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox18 fixed, firefox19 fixed, firefox20 fixed)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Assignee

Description

7 years ago
No description provided.
Assignee

Comment 1

7 years ago
currently Selenium python tests that try access cookies on the page are told that the method does not exist. 

WebDriverException: Message: {'status': 404, 'value': {'message': '/session/13/cookie not found}}
Assignee

Comment 2

7 years ago
Corresponding bug on Selenium-Proxy at https://github.com/AutomatedTester/Selenium-Proxy/issues/9
Assignee

Updated

7 years ago
Blocks: webdriver
Assignee

Updated

7 years ago
Duplicate of this bug: 810254
As Zac mentioned on the duped bug 810254 we would also need methods to remove individual and all cookies. That request should be taken care of on this bug too.
Summary: Add support for getting cookies from the page → Add support for getting and removing cookies from the page
Assignee

Updated

7 years ago
Assignee: nobody → dburns
Assignee

Comment 6

7 years ago
will add review details when try results are in
Assignee

Comment 7

7 years ago
Attachment #683590 - Attachment is obsolete: true
Assignee

Updated

7 years ago
Attachment #683606 - Flags: review?(mdas)
Assignee

Comment 9

7 years ago
ignore previous comment, was meant for another bug
Comment on attachment 683606 [details] [diff] [review]
Adding APIs to manipulate cookies

Review of attachment 683606 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/marionette/client/marionette/tests/unit/test_cookies.py
@@ +1,4 @@
> +import calendar
> +import time
> +import random
> +#from selenium._test.selenium.webmarionette.common import utils

can you remove this line?

@@ +32,5 @@
> +        cookies = self.marionette.get_cookies()
> +        self.assertEquals(0, len(cookies))
> +
> +    def testDeleteAllCookie(self):
> +        self.marionette.add_cookie(self.COOKIE_A)

You should probably add a verification step here, to make sure the cookie was in fact added.

@@ +37,5 @@
> +        self.marionette.delete_all_cookies()
> +        self.assertFalse(self.marionette.get_cookies())
> +
> +    def testDeleteCookie(self):
> +        self.marionette.add_cookie(self.COOKIE_A)

Same here

@@ +41,5 @@
> +        self.marionette.add_cookie(self.COOKIE_A)
> +        self.marionette.delete_cookie("foo")
> +        self.assertFalse(self.marionette.get_cookies())
> +
> +    def testShouldGetCookieByName(self): 

there's some extra whitespace at the end here

::: testing/marionette/marionette-listener.js
@@ +53,5 @@
>  let asyncTestTimeoutId;
>  //timer for doc changes
>  let checkTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
>  
> +

extra whitespace

@@ +798,5 @@
> +  cookie = msg.json.cookie;
> +
> +  if (!cookie.expiry) {
> +    var date = new Date();
> +    date.setYear(2030);

Even though it's still arbitrary, can we just get this year + 20 years or so? I'd rather not set it to a date that may one day have to change.

@@ +816,5 @@
> +  }
> +
> +  // The cookie's domain may include a port. Which is bad. Remove it
> +  // We'll catch ip6 addresses by mistake. Since no-one uses those
> +  // this will be okay for now.

Can we file a bug for catching ip6 addresses by mistake, and mention it here? It'll be good to keep track of this, so when the time comes, we can fix it appropriately.
Attachment #683606 - Flags: review?(mdas) → review+
Try run for 70af827ac9c6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=70af827ac9c6
Results (out of 20 total builds):
    success: 18
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dburns@mozilla.com-70af827ac9c6
I looked at this patch and don't know why it's failing on B2G.  I'd suggest adding some debugging statements to it and running it on B2G to see where it's failing.
Assignee

Updated

7 years ago
Depends on: 815616
Assignee

Comment 15

7 years ago
Pushed to try with B2G cookie tests off. https://tbpl.mozilla.org/?tree=Try&rev=abdff5001a08
Assignee

Comment 17

7 years ago
green on B2G
Try run for acd6402477e1 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=acd6402477e1
Results (out of 11 total builds):
    success: 10
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dburns@mozilla.com-acd6402477e1
Assignee

Comment 19

7 years ago
Posted patch made b2g disabled (obsolete) — Splinter Review
Disabled B2G tests due to bug 815616
Attachment #683606 - Attachment is obsolete: true
Attachment #685736 - Flags: review?(mdas)
Assignee

Updated

7 years ago
Blocks: 816301
Comment on attachment 685736 [details] [diff] [review]
made b2g disabled

Review of attachment 685736 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, I missed this review request email. Looks good to me, but it looks like Bug 815616 is now resolved, so can you remove the unit-tests.ini changes?
Attachment #685736 - Flags: review?(mdas) → review+
Assignee

Updated

7 years ago
Whiteboard: [automation-needed-in-beta][automation-needed-in-aurora]
Assignee

Comment 22

7 years ago
Posted patch updated patch that was landed (obsolete) — Splinter Review
Attachment #685736 - Attachment is obsolete: true
Backed out for:
{
07:54:12     INFO -  ERROR: testAddCookie (test_cookies.CookieTest)
07:54:12     INFO -  ----------------------------------------------------------------------
07:54:12    ERROR -  Traceback (most recent call last):
07:54:12     INFO -    File "/builds/slave/talos-slave/test/build/tests/marionette/tests/testing/marionette/client/marionette/tests/unit/test_cookies.py", line 26, in testAddCookie
07:54:12     INFO -      cookie_returned = str(self.marionette.execute_script("return document.cookie"))
07:54:12     INFO -    File "/builds/slave/talos-slave/test/build/tests/marionette/tests/testing/marionette/client/marionette/tests/unit/test_cookies.py", line 26, in testAddCookie
07:54:12     INFO -      cookie_returned = str(self.marionette.execute_script("return document.cookie"))
07:54:12     INFO -    File "/tools/python27/lib/python2.7/bdb.py", line 48, in trace_dispatch
07:54:12     INFO -      return self.dispatch_line(frame)
07:54:12     INFO -    File "/tools/python27/lib/python2.7/bdb.py", line 67, in dispatch_line
07:54:12     INFO -      if self.quitting: raise BdbQuit
07:54:12     INFO -  TEST-UNEXPECTED-FAIL | test_cookies.py CookieTest.testAddCookie | BdbQuit
}
eg https://tbpl.mozilla.org/php/getParsedLog.php?id=17672602&tree=Mozilla-Inbound#error2

https://hg.mozilla.org/integration/mozilla-inbound/rev/987a92aa6fca
Assignee

Comment 24

7 years ago
sorry, left a pdb statement from debugging bitrotting.

relanded in https://hg.mozilla.org/integration/mozilla-inbound/rev/54ec5996fb9b
Assignee

Comment 25

7 years ago
carrying mdas' review through
Attachment #689210 - Attachment is obsolete: true
Attachment #689217 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/54ec5996fb9b
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
The beta commit had some spurious changes; a merge problem, maybe?  This commit fixes that:

https://hg.mozilla.org/releases/mozilla-beta/rev/99207d3a7041
Try run for 70af827ac9c6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=70af827ac9c6
Results (out of 21 total builds):
    success: 18
    warnings: 2
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dburns@mozilla.com-70af827ac9c6
Try run for 3349232d93fc is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3349232d93fc
Results (out of 13 total builds):
    success: 11
    warnings: 1
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dburns@mozilla.com-3349232d93fc
Try run for acd6402477e1 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=acd6402477e1
Results (out of 12 total builds):
    success: 10
    warnings: 1
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dburns@mozilla.com-acd6402477e1
You need to log in before you can comment on or make changes to this bug.