Closed Bug 1250833 Opened 4 years ago Closed 4 years ago

Stop using browser.xul global specifics to devtools


(DevTools :: General, defect)

Not set


(firefox47 fixed)

Firefox 47
Tracking Status
firefox47 --- fixed


(Reporter: ochameau, Assigned: ochameau)




(1 file, 1 obsolete file)

Various code/tests currently depends on accessing some globals from browser.xul that are specific to devtools whereas most could just be fetched via a require() call.

bug 1250832 aims to remove these globals, so we should stop doing that.
Blocks: 1248348
Green try:
With bug 1250832 which actually remove all these globals.
Attached patch patch v1 (obsolete) — Splinter Review
This patch is quite simple, but do not hesitate to look at bug 1250832
where I attempt to remove all these globals.
All these globals are just lazy getter or very simple wrappers against devtools module/jsm.
So instead of depending on these globals, we just directly import/require them.

Note that in browser mochitests, globals from browser.xul were available in test scripts :o
That explains why I started importing related symbols in global scope!
Attachment #8723565 - Flags: review?(jryans)
Comment on attachment 8723565 [details] [diff] [review]
patch v1

Review of attachment 8723565 [details] [diff] [review]:

::: devtools/client/responsivedesign/resize-commands.js
@@ +89,5 @@
>  ];
>  function* gcli_cmd_resize(args, context) {
>    let browserWindow = context.environment.chromeWindow;
> +  yield ResponsiveUIManager.handleGcliCommand(browserWindow,

Nit: seems like somewhat odd indentation style, don't we usually try to keep the args aligned under each other?
Attachment #8723565 - Flags: review?(jryans) → review+
Attached patch patch v2Splinter Review
Fixed indentation.
When indentation is too far right, I tend to prefer just having one indentation for all next arguments.
But I'm not sure that's a common practice in /devtools/
Attachment #8724404 - Flags: review+
Attachment #8723565 - Attachment is obsolete: true
Bug 1250833 - Stop using browser.xul globals specific to devtools. r=jryans
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.