Closed Bug 1414053 Opened 2 years ago Closed 2 years ago

DevTools.jsm should use a custom callback instead of selectors to get the cropping region

Categories

(Testing :: mozscreenshots, defect)

Version 3
defect
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: mill2540, Assigned: mill2540)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20170926100259

Steps to reproduce:

Attempt to run any test involving DevTools.jsm on the new integration bug #1403686.


Actual results:

TestRunner.jsm fails with null element errors when it tries to the use the selectors, because the selectors can't cross the boundary into the anonymous devtools element.


Expected results:

It should use the new, more flexible selector API to use a custom callback to cross into the devtools element and run the query there.
Comment on attachment 8924714 [details]
Bug 1414053 - Adds the boundary crossing callback to DevTools.jsm.

https://reviewboard.mozilla.org/r/195934/#review201422

Thanks grenewode! Looks good - just one suggestion; see below.

::: browser/tools/mozscreenshots/mozscreenshots/extension/configurations/DevTools.jsm:32
(Diff revision 1)
> +        function() {
> +          return gDevTools.getToolbox(getTargetForSelectedTab()).win.document.querySelector("#toolbox-container");
> +        }

Could we define this function once as a property on `DevTools` and refer to it each time you're using it here to DRY this code up?
Attachment #8924714 - Flags: review?(mconley) → review-
Assignee: nobody → mill2540
Comment on attachment 8924714 [details]
Bug 1414053 - Adds the boundary crossing callback to DevTools.jsm.

https://reviewboard.mozilla.org/r/195934/#review201910

Looks great, thanks!
Attachment #8924714 - Flags: review?(mconley) → review+
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/13763ec91000
Adds the boundary crossing callback to DevTools.jsm.  r=mconley
Landed!
Flags: needinfo?(mconley)
https://hg.mozilla.org/mozilla-central/rev/13763ec91000
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.