Bug 1370467 - avoid CPOW in screenshot gcli command;r=pbro draft
authorJulian Descottes <jdescottes@mozilla.com>
Wed, 07 Jun 2017 01:13:42 +0200
changeset 590238 e907a864cf37d89250b47f843d0ee6dff3f6d66d
parent 589687 c511ad826fe71ba2bb956d2e1c119e86d2266aba
child 632139 b59de5d0e6c6a04b81efeda00e29a1f93ef8a3e6
push id62647
push userjdescottes@mozilla.com
push dateWed, 07 Jun 2017 10:21:07 +0000
reviewerspbro
bugs1370467
milestone55.0a1
Bug 1370467 - avoid CPOW in screenshot gcli command;r=pbro MozReview-Commit-ID: 9nFkfmjG8CD
devtools/shared/gcli/commands/screenshot.js
--- a/devtools/shared/gcli/commands/screenshot.js
+++ b/devtools/shared/gcli/commands/screenshot.js
@@ -42,69 +42,77 @@ const filenameParam = {
     existing: "maybe",
   },
   defaultValue: FILENAME_DEFAULT_VALUE,
   description: l10n.lookup("screenshotFilenameDesc"),
   manual: l10n.lookup("screenshotFilenameManual")
 };
 
 /**
- * Both commands have the same set of standard optional parameters
+ * Both commands have almost the same set of standard optional parameters, except for the
+ * type of the --selector option, which can be a node only on the server.
  */
-const standardParams = {
-  group: l10n.lookup("screenshotGroupOptions"),
-  params: [
-    {
-      name: "clipboard",
-      type: "boolean",
-      description: l10n.lookup("screenshotClipboardDesc"),
-      manual: l10n.lookup("screenshotClipboardManual")
-    },
-    {
-      name: "imgur",
-      type: "boolean",
-      description: l10n.lookup("screenshotImgurDesc"),
-      manual: l10n.lookup("screenshotImgurManual")
-    },
-    {
-      name: "delay",
-      type: { name: "number", min: 0 },
-      defaultValue: 0,
-      description: l10n.lookup("screenshotDelayDesc"),
-      manual: l10n.lookup("screenshotDelayManual")
-    },
-    {
-      name: "dpr",
-      type: { name: "number", min: 0, allowFloat: true },
-      defaultValue: 0,
-      description: l10n.lookup("screenshotDPRDesc"),
-      manual: l10n.lookup("screenshotDPRManual")
-    },
-    {
-      name: "fullpage",
-      type: "boolean",
-      description: l10n.lookup("screenshotFullPageDesc"),
-      manual: l10n.lookup("screenshotFullPageManual")
-    },
-    {
-      name: "selector",
-      type: "node",
-      defaultValue: null,
-      description: l10n.lookup("inspectNodeDesc"),
-      manual: l10n.lookup("inspectNodeManual")
-    },
-    {
-      name: "file",
-      type: "boolean",
-      description: l10n.lookup("screenshotFileDesc"),
-      manual: l10n.lookup("screenshotFileManual"),
-    },
-  ]
+const getScreenshotCommandParams = function (isClient) {
+  return {
+    group: l10n.lookup("screenshotGroupOptions"),
+    params: [
+      {
+        name: "clipboard",
+        type: "boolean",
+        description: l10n.lookup("screenshotClipboardDesc"),
+        manual: l10n.lookup("screenshotClipboardManual")
+      },
+      {
+        name: "imgur",
+        type: "boolean",
+        description: l10n.lookup("screenshotImgurDesc"),
+        manual: l10n.lookup("screenshotImgurManual")
+      },
+      {
+        name: "delay",
+        type: { name: "number", min: 0 },
+        defaultValue: 0,
+        description: l10n.lookup("screenshotDelayDesc"),
+        manual: l10n.lookup("screenshotDelayManual")
+      },
+      {
+        name: "dpr",
+        type: { name: "number", min: 0, allowFloat: true },
+        defaultValue: 0,
+        description: l10n.lookup("screenshotDPRDesc"),
+        manual: l10n.lookup("screenshotDPRManual")
+      },
+      {
+        name: "fullpage",
+        type: "boolean",
+        description: l10n.lookup("screenshotFullPageDesc"),
+        manual: l10n.lookup("screenshotFullPageManual")
+      },
+      {
+        name: "selector",
+        // On the client side, don't try to parse the selector as a node as it will
+        // trigger an unsafe CPOW.
+        type: isClient ? "string" : "node",
+        defaultValue: null,
+        description: l10n.lookup("inspectNodeDesc"),
+        manual: l10n.lookup("inspectNodeManual")
+      },
+      {
+        name: "file",
+        type: "boolean",
+        description: l10n.lookup("screenshotFileDesc"),
+        manual: l10n.lookup("screenshotFileManual"),
+      },
+    ]
+  };
 };
 
+const clientScreenshotParams = getScreenshotCommandParams(true);
+const serverScreenshotParams = getScreenshotCommandParams(false);
+
 exports.items = [
   {
     /**
      * Format an 'imageSummary' (as output by the screenshot command).
      * An 'imageSummary' is a simple JSON object that looks like this:
      *
      * {
      *   destinations: [ "..." ], // Required array of descriptions of the
@@ -175,17 +183,17 @@ exports.items = [
     description: l10n.lookup("screenshotDesc"),
     manual: l10n.lookup("screenshotManual"),
     returnType: "imageSummary",
     buttonId: "command-button-screenshot",
     buttonClass: "command-button command-button-invertable",
     tooltipText: l10n.lookup("screenshotTooltipPage"),
     params: [
       filenameParam,
-      standardParams,
+      clientScreenshotParams,
     ],
     exec: function (args, context) {
       // Re-execute the command on the server
       const command = context.typed.replace(/^screenshot/, "screenshot_server");
       let capture = context.updateExec(command).then(output => {
         return output.error ? Promise.reject(output.data) : output.data;
       });
 
@@ -194,17 +202,20 @@ exports.items = [
     },
   },
   {
     item: "command",
     runAt: "server",
     name: "screenshot_server",
     hidden: true,
     returnType: "imageSummary",
-    params: [ filenameParam, standardParams ],
+    params: [
+      filenameParam,
+      serverScreenshotParams,
+    ],
     exec: function (args, context) {
       return captureScreenshot(args, context.environment.document);
     },
   }
 ];
 
 /**
  * This function is called to simulate camera effects