Closed
Bug 1143908
Opened 10 years ago
Closed 7 years ago
Switching to frame by element should accept element reference instead of UUID
Categories
(Remote Protocol :: Marionette, defect, P3)
Remote Protocol
Marionette
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1410652
People
(Reporter: ato, Unassigned, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug, pi-marionette-server, Whiteboard: [lang=js])
switchToFrame takes a parameter "element" that it expects to be the UUID of the iframe element to switch to. This should instead be an element reference:
{"ELEMENT": <UUID>}
Reporter | ||
Updated•10 years ago
|
Keywords: ateam-marionette-server
Reporter | ||
Updated•10 years ago
|
Whiteboard: [good first bug][lang=js]
Reporter | ||
Comment 1•10 years ago
|
||
What we need to do is extract the element reference's value and use that to look up known elements.
The tricky part to this patch is handling backwards compatibility. Changes to the client bindings for Marionette will need to be done, but to handle backwards compatibility for some time we can look to see if msg.parameters.element is an object or a string first.
Reporter | ||
Comment 2•10 years ago
|
||
The relevant line that needs to change is in testing/marionette/marionette-listener.js: https://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js?from=marionette-listener.js#1911
Comment 3•9 years ago
|
||
(In reply to Andreas Tolfsen (:ato) from comment #2)
> The relevant line that needs to change is in
> testing/marionette/marionette-listener.js:
> https://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-
> listener.js?from=marionette-listener.js#1911
That returns "not found".
Comment 4•9 years ago
|
||
I guess you mean http://mxr.mozilla.org/mozilla-central/source/testing/marionette/listener.js#1701 , perhaps?
I wonder if this bug is still valid.
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #4)
> I guess you mean
> http://mxr.mozilla.org/mozilla-central/source/testing/marionette/listener.
> js#1701 , perhaps?
Yes I do.
> I wonder if this bug is still valid.
And yes it is to be specification compatible. Adding it as a blocker.
Comment 6•9 years ago
|
||
Is this still required after recent refactorings? If so, can you update with the necessary pieces
Flags: needinfo?(ato)
Reporter | ||
Comment 7•9 years ago
|
||
(In reply to David Burns :automatedtester from comment #6)
> Is this still required after recent refactorings? If so, can you update with
> the necessary pieces
Yes, this still is still actionable. I haven't done any work in this area.
The client currently sends a dictionary {id: <number>, element: <string>}, but according to the WebDriver specification it should only send a single "id" element which value should be type checked:
http://w3c.github.io/webdriver/webdriver-spec.html#switch-to-frame
When the client sends {id: <number|null|web element>}, the "id" field's value should be type checked:
let id = cmd.parameters.id;
// switch frame by index
if (typeof id == "number") {
…
}
// switch to current top-level browsing context
else if (id === null) {
…
}
// switch to frame by element
else if (element.isReference(id)) {
let el = elementManager.getKnownElement(id);
…
}
else {
throw new NoSuchFrameError();
}
Where element.isReference is a fictional new function:
element.isReference = function(obj) {
if (typeof obj == "undefined" || obj === null) {
return false;
}
let keys = Object.keys(obj);
return element.Key in keys || element.LegacyKey in keys;
};
The implementation shown above is straight forward. The complicating factor is that we need to manage backwards compatible with existing Python client versions. This would mean also checking the "element" field.
Maybe the following would be sufficient, but would have to be verified with a try run:
let {id: id, element: el} = cmd.parameters;
if (typeof el != "undefined") {
id = element.makeWebElement(el);
}
Flags: needinfo?(ato)
Updated•8 years ago
|
Keywords: good-first-bug
Whiteboard: [good first bug][lang=js] → [lang=js]
Mentor: ato
Updated•7 years ago
|
Priority: -- → P3
Reporter | ||
Comment 9•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #8)
> Andreas, this is also a dupe of bug 1410652 right?
Yes, looks like it!
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(ato)
Resolution: --- → DUPLICATE
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•