Bug 1363990 - New pref to beautify HTML code when copying from the inspector; r=jdescottes

Differential Revision: https://phabricator.services.mozilla.com/D21030

--HG--
extra : moz-landing-system : lando

devtools/client/inspector/markup/markup-context-menu.js

@@ -12,7 +12,6 @@ const { LocalizationHelper } = require("devtools/shared/l10n");
 
 loader.lazyRequireGetter(this, "Menu", "devtools/client/framework/menu");
 loader.lazyRequireGetter(this, "MenuItem", "devtools/client/framework/menu-item");
-loader.lazyRequireGetter(this, "copyLongString", "devtools/client/inspector/shared/utils", true);
 loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
 
 loader.lazyGetter(this, "TOOLBOX_L10N", function() {
@@ -101,21 +100,13 @@ class MarkupContextMenu {
    * Copy the innerHTML of the selected Node to the clipboard.
    */
   _copyInnerHTML() {
-    if (!this.selection.isNode()) {
-      return;
-    }
-
-    copyLongString(this.walker.innerHTML(this.selection.nodeFront));
+    this.markup.copyInnerHTML();
   }
 
   /**
    * Copy the outerHTML of the selected Node to the clipboard.
    */
   _copyOuterHTML() {
-    if (!this.selection.isNode()) {
-      return;
-    }
-
     this.markup.copyOuterHTML();
   }
 

devtools/client/inspector/markup/markup.js

@@ -23,12 +23,13 @@ const RootContainer = require("devtools/client/inspector/markup/views/root-conta
 
 loader.lazyRequireGetter(this, "MarkupContextMenu", "devtools/client/inspector/markup/markup-context-menu");
 loader.lazyRequireGetter(this, "SlottedNodeContainer", "devtools/client/inspector/markup/views/slotted-node-container");
-loader.lazyRequireGetter(this, "copyLongString", "devtools/client/inspector/shared/utils", true);
 loader.lazyRequireGetter(this, "getLongString", "devtools/client/inspector/shared/utils", true);
 loader.lazyRequireGetter(this, "openContentLink", "devtools/client/shared/link", true);
 loader.lazyRequireGetter(this, "HTMLTooltip", "devtools/client/shared/widgets/tooltip/HTMLTooltip", true);
 loader.lazyRequireGetter(this, "UndoStack", "devtools/client/shared/undo", true);
 loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
+loader.lazyRequireGetter(this, "beautify", "devtools/shared/jsbeautify/beautify");
+loader.lazyRequireGetter(this, "getTabPrefs", "devtools/shared/indentation", true);
 
 const INSPECTOR_L10N =
   new LocalizationHelper("devtools/client/locales/inspector.properties");
@@ -46,6 +47,7 @@ const DRAG_DROP_HEIGHT_TO_SPEED_MIN = 0.5;
 const DRAG_DROP_HEIGHT_TO_SPEED_MAX = 1;
 const ATTR_COLLAPSE_ENABLED_PREF = "devtools.markup.collapseAttributes";
 const ATTR_COLLAPSE_LENGTH_PREF = "devtools.markup.collapseAttributeLength";
+const BEAUTIFY_HTML_ON_COPY_PREF = "devtools.markup.beautifyOnCopy";
 
 /**
  * Vocabulary for the purposes of this file:
@@ -802,7 +804,7 @@ MarkupView.prototype = {
 
     switch (node.nodeType) {
       case nodeConstants.ELEMENT_NODE :
-        copyLongString(this.walker.outerHTML(node));
+        copyLongHTMLString(this.walker.outerHTML(node));
         break;
       case nodeConstants.COMMENT_NODE :
         getLongString(node.getNodeValue()).then(comment => {
@@ -815,6 +817,17 @@ MarkupView.prototype = {
     }
   },
 
+  /**
+   * Copy the innerHTML of the selected Node to the clipboard.
+   */
+  copyInnerHTML: function() {
+    if (!this.inspector.selection.isNode()) {
+      return;
+    }
+
+    copyLongHTMLString(this.walker.innerHTML(this.inspector.selection.nodeFront));
+  },
+
   /**
    * Given a type and link found in a node's attribute in the markup-view,
    * attempt to follow that link (which may result in opening a new tab, the
@@ -1467,12 +1480,7 @@ MarkupView.prototype = {
       walkerPromise = this.walker.innerHTML(node);
     }
 
-    return walkerPromise.then(longstr => {
-      return longstr.string().then(html => {
-        longstr.release().catch(console.error);
-        return html;
-      });
-    });
+    return getLongString(walkerPromise);
   },
 
   /**
@@ -2188,6 +2196,33 @@ MarkupView.prototype = {
   },
 };
 
+/**
+ * Copy the content of a longString containing HTML code to the clipboard.
+ * The string is retrieved, and possibly beautified if the user has the right pref set and
+ * then placed in the clipboard.
+ *
+ * @param  {Promise} longStringActorPromise
+ *         The promise expected to resolve a LongStringActor instance
+ */
+async function copyLongHTMLString(longStringActorPromise) {
+  let string = await getLongString(longStringActorPromise);
+
+  if (Services.prefs.getBoolPref(BEAUTIFY_HTML_ON_COPY_PREF)) {
+    const { indentUnit, indentWithTabs } = getTabPrefs();
+    string = beautify.html(string, {
+      // eslint-disable-next-line camelcase
+      preserve_newlines: false,
+      // eslint-disable-next-line camelcase
+      indent_size: indentWithTabs ? 1 : indentUnit,
+      // eslint-disable-next-line camelcase
+      indent_char: indentWithTabs ? "\t" : " ",
+      unformatted: [],
+    });
+  }
+
+  clipboardHelper.copyString(string);
+}
+
 /**
  * Map a number from one range to another.
  */

devtools/client/inspector/markup/test/browser.ini

@@ -99,6 +99,8 @@ skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keybo
 [browser_markup_anonymous_02.js]
 [browser_markup_anonymous_03.js]
 [browser_markup_anonymous_04.js]
+[browser_markup_copy_html.js]
+subsuite = clipboard
 [browser_markup_copy_image_data.js]
 subsuite = clipboard
 skip-if = (os == 'linux' && bits == 32 && debug) # bug 1328915, disable linux32 debug devtools for timeouts

devtools/client/inspector/markup/test/browser_markup_copy_html.js

@@ -0,0 +1,83 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test the copy inner and outer html menu options.
+
+// The nicely formatted HTML code.
+const FORMATTED_HTML =
+`<body>
+  <style>
+    div {
+      color: red;
+    }
+    span {
+      text-decoration: underline;
+    }
+  </style>
+  <div>
+    <span>
+      <em>Hello</em>
+    </span>
+  </div>
+  <script>
+    console.log("Hello!");
+  </script>
+</body>`;
+
+// The inner HTML of the body node from the code above.
+const FORMATTED_INNER_HTML = FORMATTED_HTML.replace(/<\/*body>/g, "").trim()
+                                           .replace(/^  /gm, "");
+
+// The formatted outer HTML, using tabs rather than spaces.
+const TABS_FORMATTED_HTML = FORMATTED_HTML.replace(/[ ]{2}/g, "\t");
+
+// The formatted outer HTML, using 3 spaces instead of 2.
+const THREE_SPACES_FORMATTED_HTML = FORMATTED_HTML.replace(/[ ]{2}/g, "   ");
+
+// Uglify the formatted code by removing all spaces and line breaks.
+const UGLY_HTML = FORMATTED_HTML.replace(/[\r\n\s]+/g, "");
+
+// And here is the inner html of the body node from the ugly code above.
+const UGLY_INNER_HTML = UGLY_HTML.replace(/<\/*body>/g, "");
+
+add_task(async function() {
+  // Load the ugly code in a new tab and open the inspector.
+  const { inspector } = await openInspectorForURL(
+    "data:text/html;charset=utf-8," + encodeURIComponent(UGLY_HTML));
+
+  info("Get the inner and outer html copy menu items");
+  const allMenuItems = openContextMenuAndGetAllItems(inspector);
+  const outerHtmlMenu = allMenuItems.find(({ id }) => id === "node-menu-copyouter");
+  const innerHtmlMenu = allMenuItems.find(({ id }) => id === "node-menu-copyinner");
+
+  info("Try to copy the outer html");
+  await waitForClipboardPromise(() => outerHtmlMenu.click(), UGLY_HTML);
+
+  info("Try to copy the inner html");
+  await waitForClipboardPromise(() => innerHtmlMenu.click(), UGLY_INNER_HTML);
+
+  info("Set the pref for beautifying html on copy");
+  await pushPref("devtools.markup.beautifyOnCopy", true);
+
+  info("Try to copy the beautified outer html");
+  await waitForClipboardPromise(() => outerHtmlMenu.click(), FORMATTED_HTML);
+
+  info("Try to copy the beautified inner html");
+  await waitForClipboardPromise(() => innerHtmlMenu.click(), FORMATTED_INNER_HTML);
+
+  info("Set the pref to stop expanding tabs into spaces");
+  await pushPref("devtools.editor.expandtab", false);
+
+  info("Check that the beautified outer html uses tabs");
+  await waitForClipboardPromise(() => outerHtmlMenu.click(), TABS_FORMATTED_HTML);
+
+  info("Set the pref to expand tabs to 3 spaces");
+  await pushPref("devtools.editor.expandtab", true);
+  await pushPref("devtools.editor.tabsize", 3);
+
+  info("Try to copy the beautified outer html");
+  await waitForClipboardPromise(() => outerHtmlMenu.click(), THREE_SPACES_FORMATTED_HTML);
+});

devtools/client/inspector/markup/views/text-editor.js

@@ -8,6 +8,7 @@ const { editableField } = require("devtools/client/shared/inplace-editor");
 const { LocalizationHelper } = require("devtools/shared/l10n");
 
 loader.lazyRequireGetter(this, "getAutocompleteMaxWidth", "devtools/client/inspector/markup/utils", true);
+loader.lazyRequireGetter(this, "getLongString", "devtools/client/inspector/shared/utils", true);
 
 const INSPECTOR_L10N =
   new LocalizationHelper("devtools/client/locales/inspector.properties");
@@ -42,15 +43,11 @@ function TextEditor(container, node, type) {
       if (!commit) {
         return;
       }
-      this.node.getNodeValue().then(longstr => {
-        longstr.string().then(oldValue => {
-          longstr.release().catch(console.error);
-
-          this.container.undo.do(() => {
-            this.node.setNodeValue(val);
-          }, () => {
-            this.node.setNodeValue(oldValue);
-          });
+      getLongString(this.node.getNodeValue()).then(oldValue => {
+        this.container.undo.do(() => {
+          this.node.setNodeValue(val);
+        }, () => {
+          this.node.setNodeValue(oldValue);
         });
       });
     },
@@ -98,12 +95,7 @@ TextEditor.prototype = {
   },
 
   update: function() {
-    let longstr = null;
-    this.node.getNodeValue().then(ret => {
-      longstr = ret;
-      return longstr.string();
-    }).then(str => {
-      longstr.release().catch(console.error);
+    getLongString(this.node.getNodeValue()).then(str => {
       this.value.textContent = str;
 
       const isWhitespace = !/[^\s]/.exec(str);

devtools/client/inspector/shared/utils.js

@@ -11,7 +11,6 @@ const promise = require("promise");
 loader.lazyRequireGetter(this, "KeyCodes", "devtools/client/shared/keycodes", true);
 loader.lazyRequireGetter(this, "getCSSLexer", "devtools/shared/css/lexer", true);
 loader.lazyRequireGetter(this, "parseDeclarations", "devtools/shared/css/parsing-utils", true);
-loader.lazyRequireGetter(this, "clipboardHelper", "devtools/shared/platform/clipboard");
 
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 
@@ -83,21 +82,6 @@ function blurOnMultipleProperties(cssProperties) {
   };
 }
 
-/**
- * Copy the content of a longString (via a promise resolving a
- * LongStringActor) to the clipboard.
- *
- * @param  {Promise} longStringActorPromise
- *         promise expected to resolve a LongStringActor instance
- * @return {Promise} promise resolving (with no argument) when the
- *         string is sent to the clipboard
- */
-function copyLongString(longStringActorPromise) {
-  return getLongString(longStringActorPromise).then(string => {
-    clipboardHelper.copyString(string);
-  }).catch(console.error);
-}
-
 /**
  * Create a child element with a set of attributes.
  *
@@ -228,7 +212,6 @@ function translateNodeFrontToGrip(nodeFront) {
 exports.advanceValidate = advanceValidate;
 exports.appendText = appendText;
 exports.blurOnMultipleProperties = blurOnMultipleProperties;
-exports.copyLongString = copyLongString;
 exports.createChild = createChild;
 exports.getLongString = getLongString;
 exports.getSelectorFromGrip = getSelectorFromGrip;

devtools/client/preferences/devtools-client.js

@@ -93,6 +93,9 @@ pref("devtools.markup.collapseAttributes", true);
 // Length to collapse attributes
 pref("devtools.markup.collapseAttributeLength", 120);
 
+// Whether to auto-beautify the HTML on copy.
+pref("devtools.markup.beautifyOnCopy", false);
+
 // DevTools default color unit
 pref("devtools.defaultColorUnit", "authored");