Closed Bug 855327 Opened 10 years ago Closed 10 years ago

[B2G] Add a property for getting the current active frame to marionette_client


(Testing :: Marionette, enhancement)

Gonk (Firefox OS)
Not set


(firefox23 wontfix, firefox24 wontfix, firefox25 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed


(Reporter: zcampbell, Assigned: wlach)




(1 file, 1 obsolete file)

With Marionette switching frames itself when other frames close it would be useful to be able to have a property that gives us the html element for the current iframe that Marionette is attached to.

This property would also be useful in abstracted methods in our framework where we could get the current frame and save its reference while we switch to the top level/system frame temporarily.
NB get_active_element() does not always return the current frame.
I've found another case for when this would be useful. Switching to the top frame to dismiss the keyboard via the mozKeyboard API and then switching back to the previous context. Rather than trying to work out the 'current' frame could we just store a reference for the last target of switch_to_frame?
Yes we would really like this if it's not a hard patch to do!

Dave I think that reference could go stale as there are some cases where a frame is killed by Gaia and Marionette has to attach itself to the next one to stay alive. Killing the call screen for example.
But in principal storing the current one when Marionette switches to it (not necessarily by switch_to_frame) seems sound to me.
Yes, I don't think it would be hard to add that.  I'll take a look at this next week.
Assignee: nobody → jgriffin
will's going to look at this bug next week
Assignee: jgriffin → wlachance
Attached patch Work in progress (obsolete) — Splinter Review
This works for every case except chrome, I think. Here's the script I've been using for testing with it:
Attachment #780568 - Flags: feedback?(jgriffin)
Comment on attachment 780568 [details] [diff] [review]
Work in progress

Review of attachment 780568 [details] [diff] [review]:

Looks good!  Nice and clean.  Eventually a test (for at least the local case) would be nice.

::: testing/marionette/client/marionette/
@@ +517,5 @@
>          return response
> +    def get_active_frame(self):
> +        response = self._send_message('getActiveFrame', 'value')
> +        return response

Don't we want to return an HTMLElement object, not just a uid?

  return HTMLElement(self, response)
Attachment #780568 - Flags: feedback?(jgriffin) → feedback+
This works for the chrome case as well.

I added unit tests for this, and split out the chrome switching tests into their own file (so we can run the standard frame switching stuff on firefoxos)
Attachment #780568 - Attachment is obsolete: true
Attachment #781860 - Flags: review?(jgriffin)
Comment on attachment 781860 [details] [diff] [review]

Review of attachment 781860 [details] [diff] [review]:

Awesome, thanks!

::: testing/marionette/marionette-server.js
@@ +1201,5 @@
> +    this.command_id = this.getCommandId();
> +
> +    if (this.context == "chrome") {
> +      if (this.curFrame) {
> +        let wrappedValue = this.curBrowser.elementManager.addToKnownElements(this.curFrame.frameElement);

Since we're not calling wrapValue any longer, this actually is just a uid, so maybe this variable should be something like 'frameUid'.
Attachment #781860 - Flags: review?(jgriffin) → review+

(I accidentally committed an earlier patch without the naming improvement suggested by :jgriffin, did a quick followup patch to fix that problem)
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.