Closed Bug 912244 Opened 6 years ago Closed 6 years ago

Correct implementation of "screenshot"s method "highlight" option

Categories

(Testing :: Marionette, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla26

People

(Reporter: jugglinmike, Assigned: automatedtester)

References

Details

Attachments

(2 files, 2 obsolete files)

According to the documentation of `screenshot` [1]:

> highlights – A list of HTMLElement objects to draw a red box around in the returned screenshot.

The current implementation does not behave as expected: specifying one or more valid element IDs triggers a silent error.

[1] https://marionette_client.readthedocs.org/en/latest/#marionette.Marionette.screenshot
Assignee: nobody → dburns
Blocks: 912251
Attached image Example of highlighting
Attached patch bug912244.patch (obsolete) — Splinter Review
There is an attachment of what it looks like. Try pushes in previous comment.
Attachment #799162 - Flags: review?(jgriffin)
Comment on attachment 799162 [details] [diff] [review]
bug912244.patch

Review of attachment 799162 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #799162 - Flags: review?(jgriffin) → review+
Comment on attachment 799162 [details] [diff] [review]
bug912244.patch

Review of attachment 799162 [details] [diff] [review]:
-----------------------------------------------------------------

Oh also, we should remove all the code that handles the needsOffset variable in this handler, since it's no longer used.
Attached patch bug912244-v2.patch (obsolete) — Splinter Review
corrected nits and carrying r+ forward
Attachment #799162 - Attachment is obsolete: true
Attachment #799341 - Flags: review+
Whiteboard: [checkin-needed-b2g18]
Backed out for bustage:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=69be1a2ffd39&jobname=marionette

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e1976a0ce384

Btw this was broken in the comment 1 Try push too...
Whiteboard: [checkin-needed-b2g18]
These are passing locally and failed try because of i left a pdb.

new try

https://tbpl.mozilla.org/?tree=Try&rev=997b1ffa3eb3
https://tbpl.mozilla.org/?tree=Try&rev=928b27ccadfb
issue appeared to be around resolution sizes as images were similar except for width.

corrected in https://tbpl.mozilla.org/?tree=Try&rev=d37b9454fac4
No longer blocks: 911196
carrying r+ forward
Attachment #799341 - Attachment is obsolete: true
Attachment #799501 - Flags: review+
Blocks: 912611
No longer blocks: 912251
https://hg.mozilla.org/mozilla-central/rev/a895979d4c8b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.