Bug 1363990 - New pref to beautify HTML code when copying from the inspector; r=jdescottes
authorPatrick Brosset <pbrosset@mozilla.com>
Tue, 26 Feb 2019 09:33:52 +0000
changeset 518932 b7080c6d1c050c10a2b452c44b08fe000bdfa44b
parent 518931 56a7eb5a2d6ac6353ed14a792a6ece9a9f7432c0
child 518933 079b60d94624afcc641b65667662579b5bc826ee
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1363990
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1363990 - New pref to beautify HTML code when copying from the inspector; r=jdescottes Differential Revision: https://phabricator.services.mozilla.com/D21030
devtools/client/inspector/markup/markup-context-menu.js
devtools/client/inspector/markup/markup.js
devtools/client/inspector/markup/test/browser.ini
devtools/client/inspector/markup/test/browser_markup_copy_html.js
devtools/client/inspector/markup/views/text-editor.js
devtools/client/inspector/shared/utils.js
devtools/client/preferences/devtools-client.js
--- a/devtools/client/inspector/markup/markup-context-menu.js
+++ b/devtools/client/inspector/markup/markup-context-menu.js
@@ -7,17 +7,16 @@
 "use strict";
 
 const Services = require("Services");
 const promise = require("promise");
 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() {
   return new LocalizationHelper("devtools/client/locales/toolbox.properties");
 });
 
 const INSPECTOR_L10N =
   new LocalizationHelper("devtools/client/locales/inspector.properties");
@@ -96,31 +95,23 @@ class MarkupContextMenu {
       container.copyImageDataUri();
     }
   }
 
   /**
    * 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();
   }
 
   /**
    * Copy a unique selector of the selected Node to the clipboard.
    */
   _copyUniqueSelector() {
     if (!this.selection.isNode()) {
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -18,22 +18,23 @@ const {scrollIntoViewIfNeeded} = require
 const {PrefObserver} = require("devtools/client/shared/prefs");
 const MarkupElementContainer = require("devtools/client/inspector/markup/views/element-container");
 const MarkupReadOnlyContainer = require("devtools/client/inspector/markup/views/read-only-container");
 const MarkupTextContainer = require("devtools/client/inspector/markup/views/text-container");
 const RootContainer = require("devtools/client/inspector/markup/views/root-container");
 
 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");
 
 // Page size for pageup/pagedown
 const PAGE_SIZE = 10;
 const DEFAULT_MAX_CHILDREN = 100;
 const NEW_SELECTION_HIGHLIGHTER_TIMER = 1000;
@@ -41,16 +42,17 @@ const DRAG_DROP_AUTOSCROLL_EDGE_MAX_DIST
 const DRAG_DROP_AUTOSCROLL_EDGE_RATIO = 0.1;
 const DRAG_DROP_MIN_AUTOSCROLL_SPEED = 2;
 const DRAG_DROP_MAX_AUTOSCROLL_SPEED = 8;
 const DRAG_DROP_HEIGHT_TO_SPEED = 500;
 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:
  *
  * MarkupContainer - the structure that holds an editor and its
  *  immediate children in the markup panel.
  *  - MarkupElementContainer: markup container for element nodes
  *  - MarkupTextContainer: markup container for text / comment nodes
@@ -797,30 +799,41 @@ MarkupView.prototype = {
   copyOuterHTML: function() {
     if (!this.inspector.selection.isNode()) {
       return;
     }
     const node = this.inspector.selection.nodeFront;
 
     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 => {
           clipboardHelper.copyString("<!--" + comment + "-->");
         });
         break;
       case nodeConstants.DOCUMENT_TYPE_NODE :
         clipboardHelper.copyString(node.doctypeString);
         break;
     }
   },
 
   /**
+   * 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
    * style editor or debugger).
    */
   followAttributeLink: function(type, link) {
     if (!type || !link) {
       return;
     }
@@ -1462,22 +1475,17 @@ MarkupView.prototype = {
     let walkerPromise = null;
 
     if (isOuter) {
       walkerPromise = this.walker.outerHTML(node);
     } else {
       walkerPromise = this.walker.innerHTML(node);
     }
 
-    return walkerPromise.then(longstr => {
-      return longstr.string().then(html => {
-        longstr.release().catch(console.error);
-        return html;
-      });
-    });
+    return getLongString(walkerPromise);
   },
 
   /**
    * Retrieve the outerHTML for a remote node.
    *
    * @param  {NodeFront} node
    *         The NodeFront to get the outerHTML for.
    * @return {Promise} that will be resolved with the outerHTML.
@@ -2184,16 +2192,43 @@ MarkupView.prototype = {
       return null;
     }
 
     return {parent, nextSibling};
   },
 };
 
 /**
+ * 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.
  */
 function map(value, oldMin, oldMax, newMin, newMax) {
   const ratio = oldMax - oldMin;
   if (ratio == 0) {
     return value;
   }
   return newMin + (newMax - newMin) * ((value - oldMin) / ratio);
--- a/devtools/client/inspector/markup/test/browser.ini
+++ b/devtools/client/inspector/markup/test/browser.ini
@@ -94,16 +94,18 @@ skip-if = os == "mac" # Full keyboard na
 [browser_markup_accessibility_new_selection.js]
 [browser_markup_accessibility_navigation_after_edit.js]
 skip-if = os == "mac" # Full keyboard navigation on OSX only works if Full Keyboard Access setting is set to All Control in System Keyboard Preferences
 [browser_markup_accessibility_semantics.js]
 [browser_markup_anonymous_01.js]
 [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
 [browser_markup_css_completion_style_attribute_01.js]
 [browser_markup_css_completion_style_attribute_02.js]
 [browser_markup_css_completion_style_attribute_03.js]
 [browser_markup_display_node_01.js]
 [browser_markup_display_node_02.js]
new file mode 100644
--- /dev/null
+++ b/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);
+});
--- a/devtools/client/inspector/markup/views/text-editor.js
+++ b/devtools/client/inspector/markup/views/text-editor.js
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 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");
 
 /**
  * Creates a simple text editor node, used for TEXT and COMMENT
  * nodes.
  *
@@ -37,25 +38,21 @@ function TextEditor(container, node, typ
     trigger: "dblclick",
     multiline: true,
     maxWidth: () => getAutocompleteMaxWidth(this.value, this.container.elt),
     trimOutput: false,
     done: (val, commit) => {
       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);
         });
       });
     },
     cssProperties: this.markup.inspector.cssProperties,
   });
 
   this.update();
 }
@@ -93,22 +90,17 @@ TextEditor.prototype = {
     if (value === this._selected) {
       return;
     }
     this._selected = value;
     this.update();
   },
 
   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);
       this.value.classList.toggle("whitespace", isWhitespace);
 
       const chars = str.replace(/\n/g, "⏎")
                      .replace(/\t/g, "⇥")
                      .replace(/ /g, "◦");
--- a/devtools/client/inspector/shared/utils.js
+++ b/devtools/client/inspector/shared/utils.js
@@ -6,17 +6,16 @@
 
 "use strict";
 
 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";
 
 /**
  * Called when a character is typed in a value editor.  This decides
  * whether to advance or not, first by checking to see if ";" was
  * typed, and then by lexing the input and seeing whether the ";"
  * would be a terminator at this point.
@@ -79,31 +78,16 @@ function blurOnMultipleProperties(cssPro
       if (props.length > 1) {
         e.target.blur();
       }
     }, 0);
   };
 }
 
 /**
- * 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.
  *
  * @param {Element} parent
  *        The parent node.
  * @param {string} tagName
  *        The tag name.
  * @param {object} attributes
  *        A set of attributes to set on the node.
@@ -223,14 +207,13 @@ function translateNodeFrontToGrip(nodeFr
       nodeType: nodeFront.nodeType,
     },
   };
 }
 
 exports.advanceValidate = advanceValidate;
 exports.appendText = appendText;
 exports.blurOnMultipleProperties = blurOnMultipleProperties;
-exports.copyLongString = copyLongString;
 exports.createChild = createChild;
 exports.getLongString = getLongString;
 exports.getSelectorFromGrip = getSelectorFromGrip;
 exports.promiseWarn = promiseWarn;
 exports.translateNodeFrontToGrip = translateNodeFrontToGrip;
--- a/devtools/client/preferences/devtools-client.js
+++ b/devtools/client/preferences/devtools-client.js
@@ -88,16 +88,19 @@ pref("devtools.layout.boxmodel.highlight
 pref("devtools.eyedropper.zoom", 6);
 
 // Enable to collapse attributes that are too long.
 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");
 
 // Enable the Memory tools
 pref("devtools.memory.enabled", true);
 
 pref("devtools.memory.custom-census-displays", "{}");
 pref("devtools.memory.custom-label-displays", "{}");