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

RESOLVED FIXED in Firefox 25, Firefox OS v1.1hd

Status

--
enhancement
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: zcampbell, Assigned: wlach)

Tracking

unspecified
mozilla25
Other
Gonk (Firefox OS)
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
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?
(Reporter)

Comment 3

6 years ago
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
Blocks: 889581
Created attachment 780568 [details] [diff] [review]
Work in progress

This works for every case except chrome, I think. Here's the script I've been using for testing with it:

http://pastebin.mozilla.org/2702418
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/marionette.py
@@ +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+
Created attachment 781860 [details] [diff] [review]
0001-Bug-855327-Add-capability-to-get-currently-active-fr.patch

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]
0001-Bug-855327-Add-capability-to-get-currently-active-fr.patch

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+
Inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/650a9b022b24
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ccd3c78dd50

(I accidentally committed an earlier patch without the naming improvement suggested by :jgriffin, did a quick followup patch to fix that problem)
https://hg.mozilla.org/mozilla-central/rev/650a9b022b24
https://hg.mozilla.org/mozilla-central/rev/0ccd3c78dd50
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
https://hg.mozilla.org/releases/mozilla-b2g18/rev/09cf0a874929
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ad9dcbd779cd
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox25: --- → fixed
You need to log in before you can comment on or make changes to this bug.