Closed Bug 1517196 Opened 6 years ago Closed 6 years ago

Invalid frame id doesn't cause a "no such frame" error to be raised

Categories

(Remote Protocol :: Marionette, defect, P1)

Product:
Remote Protocol
Component:
Marionette
Type:
defect

Tracking

(firefox-esr60 unaffected, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla66
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Blocks 1 open bug,
URL
)

Details

Attachments

(3 files, 1 obsolete file)

Bug 1517196 - [geckodriver] Fix unit test for invalid frame id. r?ato
(deleted), text/x-phabricator-request
Details
Bug 1517196 - [wdspec] Add tests for "Switch To Frame" command. r?ato
(deleted), text/x-phabricator-request
Details
Bug 1517196 - [marionette] Raise "no such element" error in "Switch To Frame" for unknown elements. r?ato
(deleted), text/x-phabricator-request
Details
Originally filed as: https://github.com/mozilla/geckodriver/issues/1456 When accessing the endpoint `session/048b061c-fae2-624c-b701-72c2e9f24b05/frame` and passing in an invalid frame id, we currently raise an invalid argument error. This is most likely a regression from switching to serde (bug 1396821). We should fix it in the 0.24 release of geckodriver.
This might actually be a bug in the spec. Lets follow up on the Github issue for now until it is clear.
Whatever solution we need, what we clearly miss are wdspec tests, which I will have to write here. Also I noticed a bug in the webdriver rust unit tests, which will be fixed as a ride-along.

We actually will update the WebDriver specification to fix this particular failure. As such lets write some wdspec tests to make sure we cover those invalid data type cases.

Assignee: nobody → hskupin
Blocks: webdriver
Status: NEW → ASSIGNED
Priority: -- → P1

Sounds great. I hope Google is committed to changing chromedriver
and that this doesn’t cause any user script regression, i.e. that
no one is relying on this behaviour in their tests.

Yes, we are going to change it. As such I filed https://github.com/w3c/webdriver/issues/1387. Sadly my PR was merged too early, which caused two remaining items to be left-over. Those will be fixed with https://github.com/w3c/webdriver/issues/1389.

If the frame id argument of the "Switch To Frame" command is a web element, but not a valid frame object, an "invalid argument" error needs to be thrown instead of a "no such frame" error.
Attachment #9035063 - Attachment is obsolete: true
Component: geckodriver → Marionette
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/68dbd12b8547 [marionette] Raise "no such element" error in "Switch To Frame" for unknown elements. r=ato https://hg.mozilla.org/integration/autoland/rev/227e15964de8 [geckodriver] Fix unit test for invalid frame id. r=ato https://hg.mozilla.org/integration/autoland/rev/c429964db93c [wdspec] Add tests for "Switch To Frame" command. r=ato
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14782 for changes under testing/web-platform/tests

Is this something which requires Beta backport or can it ride the trains?

status-firefox64: --- → wontfix
status-firefox65: --- → affected
status-firefox-esr60: --- → unaffected
Flags: needinfo?(hskupin)
Flags: in-testsuite+

It's not a regression, and I don't think that we have to uplift it to beta due to the code paths are rarely used.

status-firefox65: affected → wontfix
Flags: needinfo?(hskupin)
Keywords: regression
No longer blocks: 1495062
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: