Bug 1143742 - part2: multiline inplace-editor should support a maxWidth option;r=gl
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 31 Mar 2016 00:59:16 +0200
changeset 291345 02c981b1fb557cda132abfc28ed2a92116871a50
parent 291344 54e91ba66491139689a7104ad76b79be0ded042d
child 291346 ce42e52ab208465ef8bd5cb48e2fc8e1abe09501
push id74545
push userkwierso@gmail.com
push dateFri, 01 Apr 2016 23:05:42 +0000
treeherdermozilla-inbound@c410d4e20586 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1143742
milestone48.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 1143742 - part2: multiline inplace-editor should support a maxWidth option;r=gl The inplaceEditor now supports a maxWidth configuration option which can either be a number or a method returning a number. This maxWidth will be applied to the hidden element used in order to autosize the input. MozReview-Commit-ID: JTiCQ3HK5bn
devtools/client/inspector/markup/markup.js
devtools/client/inspector/rules/views/text-property-editor.js
devtools/client/shared/inplace-editor.js
devtools/client/shared/test/browser.ini
devtools/client/shared/test/browser_inplace-editor_maxwidth.js
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -2513,16 +2513,21 @@ function TextEditor(container, node, tem
 
   this.markup.template(template, this);
 
   editableField({
     element: this.value,
     stopOnReturn: true,
     trigger: "dblclick",
     multiline: true,
+    maxWidth: () => {
+      let elementRect = this.value.getBoundingClientRect();
+      let containerRect = this.container.elt.getBoundingClientRect();
+      return containerRect.right - elementRect.left - 2;
+    },
     trimOutput: false,
     done: (val, commit) => {
       if (!commit) {
         return;
       }
       this.node.getNodeValue().then(longstr => {
         longstr.string().then(oldValue => {
           longstr.release().then(null, console.error);
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -228,17 +228,19 @@ TextPropertyEditor.prototype = {
         start: this._onStartEditing,
         element: this.valueSpan,
         done: this._onValueDone,
         destroy: this.update,
         validate: this._onValidate,
         advanceChars: advanceValidate,
         contentType: InplaceEditor.CONTENT_TYPES.CSS_VALUE,
         property: this.prop,
-        popup: this.popup
+        popup: this.popup,
+        multiline: true,
+        maxWidth: () => this.container.getBoundingClientRect().width
       });
     }
   },
 
   /**
    * Get the path from which to resolve requests for this
    * rule's stylesheet.
    *
--- a/devtools/client/shared/inplace-editor.js
+++ b/devtools/client/shared/inplace-editor.js
@@ -86,16 +86,20 @@ const { findMostRelevantCssPropertyIndex
  *       focusable element.
  *    {Boolean} stopOnShiftTab:
  *       If true, shift tab will not advance the editor to the previous
  *       focusable element.
  *    {String} trigger: The DOM event that should trigger editing,
  *      defaults to "click"
  *    {Boolean} multiline: Should the editor be a multiline textarea?
  *      defaults to false
+ *    {Function or Number} maxWidth:
+ *       Should the editor wrap to remain below the provided max width. Only
+ *       available if multiline is true. If a function is provided, it will be
+ *       called when replacing the element by the inplace input.
  *    {Boolean} trimOutput: Should the returned string be trimmed?
  *      defaults to true
  *    {Boolean} preserveTextStyles: If true, do not copy text-related styles
  *              from `element` to the new input.
  *      defaults to false
  */
 function editableField(options) {
   return editableItem(options, function(element, event) {
@@ -194,16 +198,21 @@ function InplaceEditor(options, event) {
   this.doc = doc;
   this.elt.inplaceEditor = this;
 
   this.change = options.change;
   this.done = options.done;
   this.destroy = options.destroy;
   this.initial = options.initial ? options.initial : this.elt.textContent;
   this.multiline = options.multiline || false;
+  this.maxWidth = options.maxWidth;
+  if (typeof this.maxWidth == "function") {
+    this.maxWidth = this.maxWidth();
+  }
+
   this.trimOutput = options.trimOutput === undefined
                     ? true
                     : !!options.trimOutput;
   this.stopOnShiftTab = !!options.stopOnShiftTab;
   this.stopOnTab = !!options.stopOnTab;
   this.stopOnReturn = !!options.stopOnReturn;
   this.contentType = options.contentType || CONTENT_TYPES.PLAIN_TEXT;
   this.property = options.property;
@@ -213,37 +222,39 @@ function InplaceEditor(options, event) {
                           : !!options.preserveTextStyles;
 
   this._onBlur = this._onBlur.bind(this);
   this._onKeyPress = this._onKeyPress.bind(this);
   this._onInput = this._onInput.bind(this);
   this._onKeyup = this._onKeyup.bind(this);
 
   this._createInput();
+
+  // Hide the provided element and add our editor.
+  this.originalDisplay = this.elt.style.display;
+  this.elt.style.display = "none";
+  this.elt.parentNode.insertBefore(this.input, this.elt);
+
+  // After inserting the input to have all CSS styles applied, start autosizing.
   this._autosize();
-  this.inputCharWidth = this._getInputCharWidth();
 
+  this.inputCharDimensions = this._getInputCharDimensions();
   // Pull out character codes for advanceChars, listing the
   // characters that should trigger a blur.
   if (typeof options.advanceChars === "function") {
     this._advanceChars = options.advanceChars;
   } else {
     let advanceCharcodes = {};
     let advanceChars = options.advanceChars || "";
     for (let i = 0; i < advanceChars.length; i++) {
       advanceCharcodes[advanceChars.charCodeAt(i)] = true;
     }
     this._advanceChars = charCode => charCode in advanceCharcodes;
   }
 
-  // Hide the provided element and add our editor.
-  this.originalDisplay = this.elt.style.display;
-  this.elt.style.display = "none";
-  this.elt.parentNode.insertBefore(this.input, this.elt);
-
   this.input.focus();
 
   if (typeof options.selectAll == "undefined" || options.selectAll) {
     this.input.select();
   }
 
   if (this.contentType == CONTENT_TYPES.CSS_VALUE && this.input.value == "") {
     this._maybeSuggestCompletion(false);
@@ -281,16 +292,23 @@ InplaceEditor.prototype = {
     let val = this.trimOutput ? this.input.value.trim() : this.input.value;
     return val;
   },
 
   _createInput: function() {
     this.input =
       this.doc.createElementNS(HTML_NS, this.multiline ? "textarea" : "input");
     this.input.inplaceEditor = this;
+
+    if (this.multiline) {
+      // Hide the textarea resize handle.
+      this.input.style.resize = "none";
+      this.input.style.overflow = "hidden";
+    }
+
     this.input.classList.add("styleinspector-propertyeditor");
     this.input.value = this.initial;
     if (!this.preserveTextStyles) {
       copyTextStyles(this.elt, this.input);
     }
   },
 
   /**
@@ -347,16 +365,28 @@ InplaceEditor.prototype = {
       this.doc.createElementNS(HTML_NS, this.multiline ? "pre" : "span");
     this._measurement.className = "autosizer";
     this.elt.parentNode.appendChild(this._measurement);
     let style = this._measurement.style;
     style.visibility = "hidden";
     style.position = "absolute";
     style.top = "0";
     style.left = "0";
+
+    if (this.multiline) {
+      style.whiteSpace = "pre-wrap";
+      style.wordWrap = "break-word";
+      if (this.maxWidth) {
+        style.maxWidth = this.maxWidth + "px";
+        // Use position fixed to measure dimensions without any influence from
+        // the container of the editor.
+        style.position = "fixed";
+      }
+    }
+
     copyTextStyles(this.input, this._measurement);
     this._updateSize();
   },
 
   /**
    * Clean up the mess created by _autosize().
    */
   _stopAutosize: function() {
@@ -369,46 +399,59 @@ InplaceEditor.prototype = {
 
   /**
    * Size the editor to fit its current contents.
    */
   _updateSize: function() {
     // Replace spaces with non-breaking spaces.  Otherwise setting
     // the span's textContent will collapse spaces and the measurement
     // will be wrong.
-    this._measurement.textContent = this.input.value.replace(/ /g, "\u00a0");
+    let content = this.input.value;
+    let unbreakableSpace = "\u00a0";
 
-    let width = this._measurement.offsetWidth;
-    if (this.multiline) {
-      // Make sure there's some content in the current line.  This is a hack to
-      // account for the fact that after adding a newline the <pre> doesn't grow
-      // unless there's text content on the line.
-      width += 15;
-      this.input.style.height = this._measurement.offsetHeight + "px";
+    // Make sure the content is not empty.
+    if (content === "") {
+      content = unbreakableSpace;
+    }
+
+    // If content ends with a new line, add a blank space to force the autosize
+    // element to adapt its height.
+    if (content.lastIndexOf("\n") === content.length - 1) {
+      content = content + unbreakableSpace;
     }
 
-    if (width === 0) {
-      // If the editor is empty use a width corresponding to 1 character.
-      this.input.style.width = "1ch";
-    } else {
-      // Add 2 pixels to ensure the caret will be visible
-      width = width + 2;
-      this.input.style.width = width + "px";
+    if (!this.multiline) {
+      content = content.replace(/ /g, unbreakableSpace);
     }
+
+    this._measurement.textContent = content;
+
+    // Do not use offsetWidth: it will round floating width values.
+    let width = this._measurement.getBoundingClientRect().width + 2;
+    if (this.multiline) {
+      if (this.maxWidth) {
+        width = Math.min(this.maxWidth, width);
+      }
+      let height = this._measurement.getBoundingClientRect().height;
+      this.input.style.height = height + "px";
+    }
+    this.input.style.width = width + "px";
   },
 
   /**
-   * Get the width of a single character in the input to properly position the
-   * autocompletion popup.
+   * Get the width and height of a single character in the input to properly
+   * position the autocompletion popup.
    */
-  _getInputCharWidth: function() {
-    // Just make the text content to be 'x' to get the width of any character in
-    // a monospace font.
+  _getInputCharDimensions: function() {
+    // Just make the text content to be 'x' to get the width and height of any
+    // character in a monospace font.
     this._measurement.textContent = "x";
-    return this._measurement.offsetWidth;
+    let width = this._measurement.clientWidth;
+    let height = this._measurement.clientHeight;
+    return { width, height };
   },
 
    /**
    * Increment property values in rule view.
    *
    * @param {Number} increment
    *        The amount to increase/decrease the property value.
    * @return {Boolean} true if value has been incremented.
@@ -1302,20 +1345,64 @@ InplaceEditor.prototype = {
 };
 
 /**
  * Copy text-related styles from one element to another.
  */
 function copyTextStyles(from, to) {
   let win = from.ownerDocument.defaultView;
   let style = win.getComputedStyle(from);
-  to.style.fontFamily = style.getPropertyCSSValue("font-family").cssText;
-  to.style.fontSize = style.getPropertyCSSValue("font-size").cssText;
-  to.style.fontWeight = style.getPropertyCSSValue("font-weight").cssText;
-  to.style.fontStyle = style.getPropertyCSSValue("font-style").cssText;
+  let getCssText = name => style.getPropertyCSSValue(name).cssText;
+
+  to.style.fontFamily = getCssText("font-family");
+  to.style.fontSize = getCssText("font-size");
+  to.style.fontWeight = getCssText("font-weight");
+  to.style.fontStyle = getCssText("font-style");
+  to.style.lineHeight = getCssText("line-height");
+
+  // If box-sizing is set to border-box, box model styles also need to be
+  // copied.
+  let boxSizing = getCssText("box-sizing");
+  if (boxSizing === "border-box") {
+    to.style.boxSizing = boxSizing;
+    copyBoxModelStyles(from, to);
+  }
+}
+
+/**
+ * Copy box model styles that can impact width and height measurements when box-
+ * sizing is set to "border-box" instead of "content-box".
+ *
+ * @param {DOMNode} from
+ *        the element from which styles are copied
+ * @param {DOMNode} to
+ *        the element on which copied styles are applied
+ */
+function copyBoxModelStyles(from, to) {
+  let win = from.ownerDocument.defaultView;
+  let style = win.getComputedStyle(from);
+  let getCssText = name => style.getPropertyCSSValue(name).cssText;
+
+  // Copy all paddings.
+  to.style.paddingTop = getCssText("padding-top");
+  to.style.paddingRight = getCssText("padding-right");
+  to.style.paddingBottom = getCssText("padding-bottom");
+  to.style.paddingLeft = getCssText("padding-left");
+
+  // Copy border styles.
+  to.style.borderTopStyle = getCssText("border-top-style");
+  to.style.borderRightStyle = getCssText("border-right-style");
+  to.style.borderBottomStyle = getCssText("border-bottom-style");
+  to.style.borderLeftStyle = getCssText("border-left-style");
+
+  // Copy border widths.
+  to.style.borderTopWidth = getCssText("border-top-width");
+  to.style.borderRightWidth = getCssText("border-right-width");
+  to.style.borderBottomWidth = getCssText("border-bottom-width");
+  to.style.borderLeftWidth = getCssText("border-left-width");
 }
 
 /**
  * Trigger a focus change similar to pressing tab/shift-tab.
  */
 function moveFocus(win, direction) {
   return focusManager.moveFocus(win, null, direction, 0);
 }
--- a/devtools/client/shared/test/browser.ini
+++ b/devtools/client/shared/test/browser.ini
@@ -107,16 +107,17 @@ skip-if = e10s # Bug 1221911, bug 122228
 [browser_graphs-14.js]
 skip-if = e10s # Bug 1221911, bug 1222289, frequent e10s timeouts
 [browser_graphs-15.js]
 skip-if = e10s # Bug 1221911, bug 1222289, frequent e10s timeouts
 [browser_graphs-16.js]
 skip-if = e10s # Bug 1221911, bug 1222289, frequent e10s timeouts
 [browser_inplace-editor-01.js]
 [browser_inplace-editor-02.js]
+[browser_inplace-editor_maxwidth.js]
 [browser_layoutHelpers.js]
 skip-if = e10s # Layouthelpers test should not run in a content page.
 [browser_layoutHelpers-getBoxQuads.js]
 skip-if = e10s # Layouthelpers test should not run in a content page.
 [browser_mdn-docs-01.js]
 [browser_mdn-docs-02.js]
 [browser_mdn-docs-03.js]
 [browser_num-l10n.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/shared/test/browser_inplace-editor_maxwidth.js
@@ -0,0 +1,134 @@
+/* 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";
+
+var { editableField } = require("devtools/client/shared/inplace-editor");
+
+const LINE_HEIGHT = 15;
+const MAX_WIDTH = 300;
+const START_TEXT = "Start text";
+const LONG_TEXT = "I am a long text and I will not fit in a 300px container. " +
+  "I expect the inplace editor to wrap.";
+
+// Test the inplace-editor behavior with a maxWidth configuration option
+// defined.
+
+add_task(function*() {
+  yield addTab("data:text/html;charset=utf-8,inplace editor max width tests");
+  let [host, , doc] = yield createHost();
+
+  info("Testing the maxWidth option in pixels, to precisely check the size");
+  yield new Promise(resolve => {
+    createInplaceEditorAndClick({
+      multiline: true,
+      maxWidth: MAX_WIDTH,
+      start: testMaxWidth,
+      done: resolve
+    }, doc);
+  });
+
+  host.destroy();
+  gBrowser.removeCurrentTab();
+});
+
+let testMaxWidth = Task.async(function* (editor) {
+  is(editor.input.value, START_TEXT, "Span text content should be used");
+  ok(editor.input.offsetWidth < MAX_WIDTH,
+    "Input width should be strictly smaller than MAX_WIDTH");
+  is(getLines(editor.input), 1, "Input should display 1 line of text");
+
+  info("Check a text is on several lines if it does not fit MAX_WIDTH");
+  for (let key of LONG_TEXT) {
+    EventUtils.sendChar(key);
+    checkScrollbars(editor.input);
+  }
+
+  is(editor.input.value, LONG_TEXT, "Long text should be the input value");
+  is(editor.input.offsetWidth, MAX_WIDTH,
+    "Input width should be the same as MAX_WIDTH");
+  is(getLines(editor.input), 3, "Input should display 3 lines of text");
+  checkScrollbars(editor.input);
+
+  info("Delete all characters on line 3.");
+  while (getLines(editor.input) === 3) {
+    EventUtils.sendKey("BACK_SPACE");
+    checkScrollbars(editor.input);
+  }
+
+  is(editor.input.offsetWidth, MAX_WIDTH,
+    "Input width should be the same as MAX_WIDTH");
+  is(getLines(editor.input), 2, "Input should display 2 lines of text");
+  checkScrollbars(editor.input);
+
+  info("Delete all characters on line 2.");
+  while (getLines(editor.input) === 2) {
+    EventUtils.sendKey("BACK_SPACE");
+    checkScrollbars(editor.input);
+  }
+
+  is(getLines(editor.input), 1, "Input should display 1 line of text");
+  checkScrollbars(editor.input);
+
+  info("Delete all characters.");
+  while (editor.input.value !== "") {
+    EventUtils.sendKey("BACK_SPACE");
+    checkScrollbars(editor.input);
+  }
+
+  ok(editor.input.offsetWidth < MAX_WIDTH,
+    "Input width should again be strictly smaller than MAX_WIDTH");
+  ok(editor.input.offsetWidth > 0,
+    "Even with no content, the input has a non-zero width");
+  is(getLines(editor.input), 1, "Input should display 1 line of text");
+  checkScrollbars(editor.input);
+
+  info("Leave the inplace-editor");
+  EventUtils.sendKey("RETURN");
+});
+
+/**
+ * Retrieve the current number of lines displayed in the provided textarea.
+ *
+ * @param {DOMNode} textarea
+ * @return {Number} the number of lines
+ */
+function getLines(textarea) {
+  return Math.floor(textarea.clientHeight / LINE_HEIGHT);
+}
+
+/**
+ * Verify that the provided textarea has no vertical or horizontal scrollbar.
+ *
+ * @param {DOMNode} textarea
+ */
+function checkScrollbars(textarea) {
+  is(textarea.scrollHeight, textarea.clientHeight,
+    "Textarea should never have vertical scrollbars");
+  is(textarea.scrollWidth, textarea.clientWidth,
+    "Textarea should never have horizontal scrollbars");
+}
+
+function createInplaceEditorAndClick(options, doc) {
+  doc.body.innerHTML = "";
+  let span = options.element = createSpan(doc);
+
+  info("Creating an inplace-editor field");
+  editableField(options);
+
+  info("Clicking on the inplace-editor field to turn to edit mode");
+  span.click();
+}
+
+function createSpan(doc) {
+  info("Creating a new span element");
+  let span = doc.createElement("span");
+  span.setAttribute("tabindex", "0");
+  span.style.lineHeight = LINE_HEIGHT + "px";
+  span.style.fontSize = "11px";
+  span.style.fontFamily = "monospace";
+  span.textContent = START_TEXT;
+  doc.body.appendChild(span);
+  return span;
+}