Closed Bug 754220 Opened 9 years ago Closed 8 years ago

Add support for getting and removing cookies from the page

Categories

(Testing :: Marionette, defect)

defect
Not set
normal

Tracking

(firefox18 fixed, firefox19 fixed, firefox20 fixed)

RESOLVED FIXED
mozilla20
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: automatedtester, Assigned: automatedtester)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

No description provided.
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}}
Corresponding bug on Selenium-Proxy at https://github.com/AutomatedTester/Selenium-Proxy/issues/9
Blocks: webdriver
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: nobody → dburns
will add review details when try results are in
Attachment #683590 - Attachment is obsolete: true
Attachment #683606 - Flags: review?(mdas)
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.
Depends on: 815616
Pushed to try with B2G cookie tests off. https://tbpl.mozilla.org/?tree=Try&rev=abdff5001a08
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
Attached 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)
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+
Whiteboard: [automation-needed-in-beta][automation-needed-in-aurora]
Attached 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
sorry, left a pdb statement from debugging bitrotting.

relanded in https://hg.mozilla.org/integration/mozilla-inbound/rev/54ec5996fb9b
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
Closed: 8 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.