977899 - get() in marionette currently can navigate the frame instead of the top level browsing context.

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P3)

Description

[David Burns :automatedtester]

When we do get() we should be navigating at the top level browsing context, as per webdriver spec. Current behaviour has it happening at the frame level

Comment 1

[David Burns :automatedtester]

Attachment: MozReview Request: Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`

Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`
is called. r?ato

This means that we are now doing step 7 in

http://w3c.github.io/webdriver/webdriver-spec.html#get

Comment 2

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8670231 [details]
MozReview Request: Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`

https://reviewboard.mozilla.org/r/21345/#review19235

::: testing/marionette/client/marionette/tests/unit/test_switch_frame.py:128
(Diff revision 1)
> +    def test_navigation_after_switching_to_child_frame_navigates_top(self):

What is a child frame as opposed to just a frame?  Can we make the test name shorter?

::: testing/marionette/listener.js:1328
(Diff revision 1)
> +  if (isB2G){

Space before `{`.

::: testing/marionette/listener.js:1330
(Diff revision 1)
> -}
> +  }
> +  else {

Put on same line

::: testing/marionette/listener.js:1333
(Diff revision 1)
> +    sendSyncMessage("Marionette:switchedToFrame", { frameValue: null });

This API is really horrible, whereby a certain behaviour happens if we pass along null.

(Not opening an issue on this, just pointing it out.)

Comment 3

[David Burns :automatedtester]

Comment on attachment 8670231 [details]
MozReview Request: Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`

Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`
is called. r?ato

This means that we are now doing step 7 in

http://w3c.github.io/webdriver/webdriver-spec.html#get

Comment 4

[David Burns :automatedtester]

Comment on attachment 8670231 [details]
MozReview Request: Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`

Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`
is called. r?ato

This means that we are now doing step 7 in

http://w3c.github.io/webdriver/webdriver-spec.html#get

Comment 5

[David Burns :automatedtester]

https://reviewboard.mozilla.org/r/21345/#review19235

> What is a child frame as opposed to just a frame?  Can we make the test name shorter?

Updated the test name to be slightly more meaningful by removing some redunduncy. Child indicates that we shouldnt be doing `switchToFrame()` in the test.

Making the test shorter would remove some of the meaning. I have a great dislike for tests with method names like `def test_frame_navigate(self)` as they have little to no indication of what they should be doing

Comment 6

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8670231 [details]
MozReview Request: Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`

https://reviewboard.mozilla.org/r/21345/#review19309

I couldn’t find a try run for this patch, but the code looks sound to me.

Comment 7

[David Burns :automatedtester]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ebe60391a8be11dbdcfd830b4ebbf04e50ba0d20
Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ebe60391a8be

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e8b417baee24