Closed Bug 1605327 Opened 1 year ago Closed 1 year ago

Deprecate cd() command

Categories

(DevTools :: Console, enhancement, P1)

enhancement

Tracking

(Fission Milestone:Future, firefox74 fixed)

RESOLVED FIXED
Firefox 74
Fission Milestone Future
Tracking Status
firefox74 --- fixed

People

(Reporter: Honza, Assigned: nchevobbe)

References

(Blocks 1 open bug)

Details

(Whiteboard: dt-fission-m2-mvp)

Attachments

(2 files)

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

Fission Milestone: --- → Future

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?

Flags: needinfo?(odvarko)

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.

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

Honza

Flags: needinfo?(odvarko)
Attached image 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.

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

Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1
Blocks: 1607741

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.

LGTM. Localization is probably not needed.

Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e00059f0bcff
Add a deprecation warning when the cd command is called. r=Honza.
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74
You need to log in before you can comment on or make changes to this bug.