Closed Bug 977899 Opened 6 years ago Closed 4 years ago

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

Categories

(Testing :: Marionette, defect, P3)

x86
macOS
defect

Tracking

(firefox43 fixed, firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
firefox43 --- fixed
firefox44 --- fixed

People

(Reporter: automatedtester, Unassigned)

References

()

Details

(Keywords: pi-marionette-spec)

Attachments

(1 file)

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
Whiteboard: [spec]
Priority: -- → P3
Whiteboard: [spec]
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
Attachment #8670231 - Flags: review?(ato)
Attachment #8670231 - Flags: review?(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 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
Attachment #8670231 - Flags: review?(ato)
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
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
Attachment #8670231 - Flags: review?(ato) → review+
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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebe60391a8be11dbdcfd830b4ebbf04e50ba0d20
Bug 977899: Have Marionette navigate the top frame on desktop when `navigate()`
https://hg.mozilla.org/mozilla-central/rev/ebe60391a8be
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Whiteboard: [checkin-needed-aurora]
You need to log in before you can comment on or make changes to this bug.