Closed Bug 1108590 Opened 7 years ago Closed 7 years ago

Update key in object returned in findElement to match spec

Categories

(Testing :: Marionette, defect)

x86
macOS
defect
Not set
normal

Tracking

(firefox39 fixed)

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: automatedtester, Assigned: automatedtester)

References

Details

(Keywords: pi-marionette-server, pi-marionette-spec)

Attachments

(2 files, 1 obsolete file)

Currently we return

{ "ELEMENT": "<some uuid>"}

THe spec has been updated to use element-6066-11e4-a52e-4f735466cecf instead of ELEMENT.
Blocks: 1137972
Assignee: nobody → dburns
/r/4535 - Bug 1108590: Update key that is used to return elements from marionette to the client; r=chmanchester

Pull down this commit:

hg pull review -r 1e3e64cce28043e5fa22fa15bdbf023c1de75e9c
Attachment #8571982 - Flags: review?(cmanchester)
Comment on attachment 8571982 [details]
MozReview Request: bz://1108590/AutomatedTester

https://reviewboard.mozilla.org/r/4533/#review3777

::: testing/marionette/driver/marionette_driver/marionette.py
(Diff revision 1)
> -            wrapped = {'ELEMENT': args.id }
> +            wrapped = {'element-6066-11e4-a52e-4f735466cecf': args.id }

Any reason not to accept either key for a while so we can work with slightly older geckos? Maybe that's a lost cause at this point.

::: testing/marionette/driver/marionette_driver/marionette.py
(Diff revision 1)
> +                    return unwrapped

Breaking out of the loop might be clearer than another return.

::: testing/marionette/marionette-elements.js
(Diff revision 1)
> +          result = {'ELEMENT': elementId, 'element-6066-11e4-a52e-4f735466cecf': elementId};

Maybe make the new key for elements a property of the element manager.

::: testing/marionette/marionette-elements.js
(Diff revision 1)
> -        else if (typeof(args['ELEMENT'] === 'string') &&
> -                 args.hasOwnProperty('ELEMENT')) {
> +        else if ((typeof(args['ELEMENT'] === 'string') && args.hasOwnProperty('ELEMENT')) ||
> +                 (typeof(args['element-6066-11e4-a52e-4f735466cecf'] === 'string') &&

This was here before, but the parens after these typeofs seem misplaced, these checks will always evaluate to the string "boolean", and be truthy.

::: testing/marionette/marionette-elements.js
(Diff revision 1)
> -          result = {'ELEMENT': this.addToKnownElements(val)};
> +          var elementId = this.addToKnownElements(val);

s/var/let/

::: testing/marionette/marionette-elements.js
(Diff revision 1)
> +          var elementUniqueIdentifier = args['element-6066-11e4-a52e-4f735466cecf'] ? args['element-6066-11e4-a52e-4f735466cecf'] : args['ELEMENT'];

s/var/let/
Comment on attachment 8571982 [details]
MozReview Request: bz://1108590/AutomatedTester

/r/4535 - Bug 1108590: Update key that is used to return elements from marionette to the client; r=chmanchester
/r/4809 - review fixups, this will be squashed before pushing to inbound

Pull down these commits:

hg pull review -r 024d6afec6494cd984d0d4e214b0d1f91f76ed4b
Attachment #8571982 - Flags: review?(cmanchester)
Attachment #8571982 - Flags: review?(cmanchester) → review+
https://hg.mozilla.org/mozilla-central/rev/2bb23cc942a1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Attachment #8571982 - Attachment is obsolete: true
Attachment #8618849 - Flags: review+
Attachment #8618850 - Flags: review+
You need to log in before you can comment on or make changes to this bug.