For unsupported commands in chrome context throw UnsupportedOperationError

RESOLVED FIXED in Firefox 52

Status

Testing
Marionette
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

(Blocks: 1 bug)

Version 3
mozilla53
Points:
---

Firefox Tracking Flags

(firefox52 fixed, firefox53 fixed)

Details

(Whiteboard: [spec][17/01])

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

a year ago
Some of the webdriver commands do not throw a UnsupportedOperationError exception in case the command gets issued in chrome context.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8822419 - Flags: review?(ato)

Comment 3

a year ago
mozreview-review
Comment on attachment 8822419 [details]
Bug 1326174 - For unsupported commands in chrome context throw UnsupportedOperationError

https://reviewboard.mozilla.org/r/101338/#review101902

::: testing/marionette/assert.js:81
(Diff revision 2)
> + *
> + * @throws {UnsupportedOperationError}
> + *     If |context| is not content.
> + */
> +assert.content = function (context, msg = "") {
> +  msg = msg || error.pprint`Expected ${context} to be content`;

I wonder if this should simply say “Command not supported in chrome context” so we don’t have to override the message every time.  I don’t think there is any value in saying which _specific_ command was not available.

::: testing/marionette/driver.js:2530
(Diff revision 2)
>    ];
>  
>    let or = String(cmd.parameters.orientation);
>    assert.string(or);
>    let mozOr = or.toLowerCase();
> -  if (!ors.include(mozOr)) {
> +  if (!ors.includes(mozOr)) {

Well spotted.
Attachment #8822419 - Flags: review?(ato) → review+
(Assignee)

Comment 4

a year ago
mozreview-review-reply
Comment on attachment 8822419 [details]
Bug 1326174 - For unsupported commands in chrome context throw UnsupportedOperationError

https://reviewboard.mozilla.org/r/101338/#review101902

> I wonder if this should simply say “Command not supported in chrome context” so we don’t have to override the message every time.  I don’t think there is any value in saying which _specific_ command was not available.

That makes sense. I will correct it.

> Well spotted.

Well, it was hiding for Fennec until I landed my skip decorator fix earlier today. So I have seen it here:

https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=mn&bugfiler&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=65287721

I actually may want to split this out to a separate commit.
(Assignee)

Comment 5

a year ago
mozreview-review-reply
Comment on attachment 8822419 [details]
Bug 1326174 - For unsupported commands in chrome context throw UnsupportedOperationError

https://reviewboard.mozilla.org/r/101338/#review101902

> That makes sense. I will correct it.

I assume you are fine if I also correct this for assert.[firefox,fennec,mobile].
Comment hidden (mozreview-request)
(Assignee)

Comment 7

a year ago
(In reply to Henrik Skupin (:whimboo) from comment #5)
> I assume you are fine if I also correct this for
> assert.[firefox,fennec,mobile].

Can you please have a look again? Thanks.
Flags: needinfo?(ato)
r++
Flags: needinfo?(ato)

Comment 9

a year ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/57a62a57c996
For unsupported commands in chrome context throw UnsupportedOperationError r=ato
(Assignee)

Comment 11

a year ago
So the utils.py Puppeteer unit test indeed failed because it was using the delete_all_cookies() method in chrome scope. I fixed it locally and will push another try build.
Flags: needinfo?(hskupin)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8822532 - Flags: review?(mjzffr)

Comment 14

a year ago
mozreview-review
Comment on attachment 8822532 [details]
Bug 1326174 - Handle cookies with content scope in test_utils.py

https://reviewboard.mozilla.org/r/101408/#review102038
Attachment #8822532 - Flags: review?(mjzffr) → review+

Comment 15

a year ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2cd1da60be54
Handle cookies with content scope in test_utils.py r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/3e78b4df8dd7
For unsupported commands in chrome context throw UnsupportedOperationError r=ato

Comment 16

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2cd1da60be54
https://hg.mozilla.org/mozilla-central/rev/3e78b4df8dd7
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Comment 17

a year ago
Test-only patch which we need to uplift to aurora. Thanks.
status-firefox52: --- → affected
Whiteboard: [checkin-needed-aurora]
the 2nd patch need rebasing for aurora
Flags: needinfo?(hskupin)
(Assignee)

Comment 19

a year ago
(In reply to Carsten Book [:Tomcat] from comment #18)
> the 2nd patch need rebasing for aurora

I pulled latest aurora code and tried to graft both commits which worked fine. Can you please check again what was wrong with your graft command?
Flags: needinfo?(hskupin)

Comment 20

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/4dcafa858f95
https://hg.mozilla.org/releases/mozilla-aurora/rev/b910341ba56c
status-firefox52: affected → fixed
Whiteboard: [checkin-needed-aurora]
(Assignee)

Updated

a year ago
Whiteboard: [spec][17/01]
You need to log in before you can comment on or make changes to this bug.