Closed Bug 1301073 Opened 8 years ago Closed 3 years ago

WebDriver:SwitchToWindow should not allow switching by window name

Categories

(Testing :: geckodriver, defect, P3)

Version 3
defect

Tracking

(firefox95 fixed)

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: standard8, Assigned: whimboo)

References

(Blocks 1 open bug)

Details

(Keywords: pi-marionette-spec)

Attachments

(1 file)

The w3c specification for WebDriver [1] doesn't allow for passing a window "name" as a handle to switchToWindow - only an id. Currently the code allows for a "name" or an "id", however the implementation of "name" is wrong as it uses the window name of the actual window rather than the window.name of the content process within a tab (xref bug 1283941). Seeing as it doesn't need to be supported, we should just remove the name option to save confusion. [1] https://w3c.github.io/webdriver/webdriver-spec.html#switch-to-window
Blocks: webdriver
Summary: switchToWindow shouldn't take a handle as a name. → switchToWindow shouldn't take a handle as a name
OS: Unspecified → All
Hardware: Unspecified → All
Summary: switchToWindow shouldn't take a handle as a name → Switch To Window should not allow switching by window name
Priority: -- → P2
Summary: Switch To Window should not allow switching by window name → WebDriver:SwitchToWindow should not allow switching by window name
Depends on: 1519053
Depends on: 1519055
Depends on: 1520585, 1534702

The necessary code change this bug covers are:

JS: https://searchfox.org/mozilla-central/rev/5e830ac8f56fe191cb58a264e01cdbf6b6e847bd/testing/marionette/driver.js#1542-1577
Py: https://searchfox.org/mozilla-central/rev/5e830ac8f56fe191cb58a264e01cdbf6b6e847bd/testing/marionette/client/marionette_driver/marionette.py#1346-1357
Rust: https://searchfox.org/mozilla-central/rev/5e830ac8f56fe191cb58a264e01cdbf6b6e847bd/testing/geckodriver/src/marionette.rs#1667-1674

Note that for geckodriver the handle argument has been added via bug 1519053 to geckodriver 0.25.0. So if we make the change to driver.js now, it would mean that everything before 0.25.0 (which only supports name) will not work with Firefox 71 and later anymore. But geckodriver 0.25.0 and later which support both name and handle will not require us to bump the minimum supported version of Firefox.

Another solution would be to add the support for handle in driver.js first, and get this released. And then we follow-up later (when we have the time for) with the name removal.

Given the actual problems with 0.25.0 with MSVCRT we may want to do the interim solution and give people more time in case they want/have to stay with geckodriver 0.24.0 for now?

Personally I would prefer the latter option. Maja and Andreas, what do you think?

Flags: needinfo?(mjzffr)
Flags: needinfo?(ato)

If switching by name breaks backwards compatibility I would prefer
not removing this feature.

Flags: needinfo?(ato)

Ok, so in that case I will file a bug for Marionette which simply adds using the handle if specified for now. Then after a couple of Firefox releases (maybe after the next ESR) we can work on this bug, and get it removed.

Flags: needinfo?(mjzffr)
Depends on: 1588424

As filed this is more a geckodriver related bug. So I'm going to move it into the correct component.

Note that with bug 1588424 fixed we no longer accept name as argument for WebDriver:SwitchToWindow in Marionette. The remaining patch to do will be to finally remove passing in the name by geckodriver:

https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/testing/geckodriver/src/marionette.rs#1720-1723

By doing that it would move the minimal supported version of Firefox to 67.0 for the geckodriver release including this change. We can easily do that for the 0.28 release.

Blocks: 1649094
Component: Marionette → geckodriver

Turns out that we actually didn't swap the Marionette code from name to handle earlier on, and my fix on bug 1588424 made the hard switch. It means that the minimum version of Firefox as supported will not be 67.0 but 80.0.

That means that we will still have to wait a while (at least until 78.0esr is no longer supported) until we can get it removed in geckodriver. Nevertheless I will attach a WIP patch.

Starting with geckodriver 0.25.0 the "handle" argument (available
since Firefox 67) for the "WebDriver:SwitchToWindow" call to
Marionette has been added. Since then the "name" argument in
Marionette has been removed (Firefox 80).

That means sending "name" is no longer necessary, and can be
removed. By doing that the minimum supported Firefox version
will be bumped to Firefox 80.0.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Priority: P2 → P3
No longer blocks: 1649094

We should at least wait with landing this patch until the next ESR will be released, and that will happen with 91.0 by mid of this year. Otherwise we would break users running ESR78.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/089fc7a7d0dd [geckodriver] Don't send "name" argument for "WebDriver:SwitchToWindow" to Marionette. r=webdriver-reviewers,jgraham
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: