Closed Bug 961767 Opened 9 years ago Closed 9 years ago

Remove dead toggleWebConsole method from hudservice.js

Categories

(DevTools :: Console, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 29

People

(Reporter: rcampbell, Assigned: rcampbell)

Details

Attachments

(1 file, 1 obsolete file)

Method's only used in a test. We should remove it.
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
Attached patch removeToggleWebConsole (obsolete) — Splinter Review
Attachment #8362608 - Flags: review?(mihai.sucan)
Comment on attachment 8362608 [details] [diff] [review]
removeToggleWebConsole

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

Since we are doing this here now... I have to also mention getOpenWebConsole() as obsolete (no users of this method anymore, according to mxr).

Thank you for the patch! r+ with the comments addressed.

::: browser/devtools/scratchpad/test/browser_scratchpad_wrong_window_focus.js
@@ +3,5 @@
>     http://creativecommons.org/publicdomain/zero/1.0/ */
>  /* Bug 661762 */
>  
> +
> +/* lifted and modified from hudservice.js */

Is this needed?

@@ +6,5 @@
> +
> +/* lifted and modified from hudservice.js */
> +function toggleWebConsole()
> +{
> +  let devtools = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}).devtools;

nit: let {devtools} = Cu.import("...", {});

@@ +7,5 @@
> +/* lifted and modified from hudservice.js */
> +function toggleWebConsole()
> +{
> +  let devtools = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}).devtools;
> +  let hud = HUDService;

Is this used?

@@ +8,5 @@
> +function toggleWebConsole()
> +{
> +  let devtools = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}).devtools;
> +  let hud = HUDService;
> +  let window = gBrowser.ownerGlobal;

Is this needed? you can directly use gBrowser.selectedTab when you call TargetFactory.forTab().

@@ +12,5 @@
> +  let window = gBrowser.ownerGlobal;
> +  let target = devtools.TargetFactory.forTab(window.gBrowser.selectedTab);
> +  let toolbox = gDevTools.getToolbox(target);
> +  return toolbox && toolbox.currentToolId == "webconsole" ?
> +    toolbox.destroy() :

Is this needed by the test? No toggle behavior, as I noticed, only one open.

@@ +54,4 @@
>          Services.obs.
>            addObserver(onWebConsoleOpen, "web-console-created", false);
>  
> +        toggleWebConsole();

Instead of the notification observer, you could use:

openWebConsole().then((toolbox) => {
  let hud = toolbox.getCurrentPanel().hud;
  testFocus(sw, hud);
});
Attachment #8362608 - Flags: review?(mihai.sucan) → review+
Fixed up and working. Thanks for the suggestions.
Attachment #8362608 - Attachment is obsolete: true
Attachment #8363181 - Flags: review+
Attachment #8363181 - Flags: checkin?
https://hg.mozilla.org/integration/fx-team/rev/31dca1ad8cfb
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/31dca1ad8cfb
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Wait!  I use this method in my OpenTrack extension.  Most of my users are students who have a hard time remembering CTRL-SHIFT-K.  I know.  I'm late to the table, but I only noticed that it broke when I upgraded to FF29.0 this morning.  Is there an alternative way to open a console via JS?  Did the suggested openWebConsole() get implemented?
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.