Closed Bug 1477475 Opened 2 years ago Closed 2 years ago

Drop legacy capabilities propagation to Marionette in geckodriver

Categories

(Testing :: geckodriver, enhancement)

enhancement
Not set

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: ato, Assigned: ato)

Details

Attachments

(1 file)

geckodriver currently writes a legacy capabilities structure to the
WebDriver:NewSession command it uses on creating a new WebDriver
session which duplicates the top-level object:

> {capabilities: {<caps>, desiredCapabilities: <caps>}}

Where caps can look something like:

> {acceptInsecureCerts: true, pageLoadStrategy: "normal", …}

Marionette has picked up the top-level "capabilities" key for a
very long time now, and there is no longer any need to duplicate
the body.
Assignee: nobody → ato
Status: NEW → ASSIGNED
geckodriver currently writes a legacy capabilities structure to the
WebDriver:NewSession command it uses on creating a new WebDriver
session which duplicates the top-level object:

	{capabilities: {<caps>, desiredCapabilities: <caps>}}

Where caps can look something like:

	{acceptInsecureCerts: true, pageLoadStrategy: "normal", ...}

Marionette has picked up the top-level "capabilities" key for a
very long time now, and there is no longer any need to duplicate
the body.
Attachment #8993909 - Flags: review?(dburns)
Comment on attachment 8993909 [details] [diff] [review]
Drop legacy Marionette capabilities duplication. r=automatedtester

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

So what about the `webdriver` crate where this is still present? I assume it has to also be removed there?

Please also update the TraceLog.md file to get rid of any `desiredCapabilities` reference.
Attachment #8993909 - Flags: review-
Attachment #8993909 - Flags: review?(dburns)
(In reply to Henrik Skupin (:whimboo) from comment #3)

> So what about the `webdriver` crate where this is still present? I
> assume it has to also be removed there?

What is still present in the webdriver crate that I remove here?

> Please also update the TraceLog.md file to get rid of any
> `desiredCapabilities` reference.

What do you mean? desiredCapabilities is still a thing that we
support.  This patch doesn’t drop support for it.
Flags: needinfo?(hskupin)
Oh, so we only want to drop the duplication for the Marionette protocol. Looks like I missed that, and now I even see that this is listed in the summary. 

Just to clarify, whatever is specified on the WebDriver part (desired vs alwaysMatch/firstMatch) we always send duplicated capabilities to Marionette?
Flags: needinfo?(hskupin)
(In reply to Henrik Skupin (:whimboo) from comment #5)

> Oh, so we only want to drop the duplication for the Marionette
> protocol.  Looks like I missed that, and now I even see that this
> is listed in the summary.

Oh sorry.  I could’ve explained that more clearly.  I see the
original description is somewhat misleading.

> Just to clarify, whatever is specified on the WebDriver part
> (desired vs alwaysMatch/firstMatch) we always send duplicated
> capabilities to Marionette?

Marionette only gets the _negotiated/matched_ capabilities, we never
send it any of the _requested_ capabilities we receive from the
user.

However because Marionette used to look for the desiredCapabilities
property on the passed object and later moved to just look at the
object itself, we duplicated desiredCapabilities inside it, so that
we ended up with something like this:

	let capabilities = {pageLoadStrategy: "normal", {desiredCapabilities: {pageLoadStrategy: "normal"}}}

Does that make sense?
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/100697883c7d
Drop legacy Marionette capabilities duplication. r=whimboo
https://hg.mozilla.org/mozilla-central/rev/100697883c7d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.