1605327 - Deprecate cd() command

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

We have the Context Selector UI now so, we might deprecate and remove this command.

Implementing cd() that would cross origin boundary does not seem feasible. I.e. we can't support "cd(foo); log(window);" universally. I.e. cd is more of a UI switch rather than a console command.

See more:
https://bugs.chromium.org/p/chromium/issues/detail?id=80926

Honza

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Honza, were you thinking of having a warning message before deprecating the command?
If so, should we I guess it should point to this bug?

Comment 2

[:Harald Kirschner :digitarald]

Jumping in for Honza, but would love to hear his thoughts as well: A console warning when executing the command would be nice, but just with minimal effort as we don't have any indication that this is widely used and because context switching UI will serve most use cases that cd() covered.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Agreed, let's start with a warning (pointing to this bug report) and perhaps also collect some feedback related to this decision.

Honza

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: image.png

we can have something very easily.
the patch is quite small:

diff --git a/devtools/server/actors/webconsole/utils.js b/devtools/server/actors/webconsole/utils.js
--- a/devtools/server/actors/webconsole/utils.js
+++ b/devtools/server/actors/webconsole/utils.js
@@ -4,7 +4,7 @@
 
 "use strict";
 
-const { Ci, Cu } = require("chrome");
+const { Ci, Cu, Cc } = require("chrome");
 
 // Note that this is only used in WebConsoleCommands, see $0, screenshot and pprint().
 if (!isWorker) {
@@ -557,6 +557,22 @@ WebConsoleCommands._registerOriginal("he
  *        eval scope is cleared back to its default (the top window).
  */
 WebConsoleCommands._registerOriginal("cd", function(owner, window) {
+  // Log a deprecation warning.
+  const scriptErrorClass = Cc["@mozilla.org/scripterror;1"];
+  const scriptError = scriptErrorClass.createInstance(Ci.nsIScriptError);
+  scriptError.initWithWindowID(
+    "The `cd` command will be deprecated in future version of Firefox. See https://bugzilla.mozilla.org/show_bug.cgi?id=1605327 for more information",
+    null,
+    null,
+    0,
+    0,
+    1,
+    "content javascript",
+    owner.window.windowUtils.currentInnerWindowID
+  );
+  const Services = require("Services");
+  Services.console.logMessage(scriptError);
+
   if (!window) {
     owner.consoleActor.evalWindow = null;
     owner.helperResult = { type: "cd" };

we might want to localize the message (or not), but this shouldn't be too hard.

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

great, I'll clean this up and push the patch

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1605327 - Add a deprecation warning when the cd command is called. r=Honza.

This is a simple patch that logs a message in the console
when the cd command is called.
The localized string is added into the shared.properties
file because there's no file dedicated to the console that
can be loaded from the server, and the message will only be
here for a cycle before being removed, so it's not really
worth it adding a new file.

Comment 7

[:Harald Kirschner :digitarald]

LGTM. Localization is probably not needed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e00059f0bcff
Add a deprecation warning when the cd command is called. r=Honza.

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/e00059f0bcff