Bug 1431071 - Gridline names suggestions offered in inspector autocomplete if element is in a grid and rule applies. r?gl, jdescottes draft
authorErica Wright <ewright@mozilla.com>
Wed, 04 Apr 2018 14:01:19 -0400
changeset 780666 76050ccc8ec04075c1eb9e10f3ef7b89227dec5a
parent 780587 cfe6399e142c71966ef58a16cfd52c0b46dc6b1e
push id106074
push userbmo:ewright@mozilla.com
push dateWed, 11 Apr 2018 19:18:34 +0000
reviewersgl, jdescottes
bugs1431071
milestone61.0a1
Bug 1431071 - Gridline names suggestions offered in inspector autocomplete if element is in a grid and rule applies. r?gl, jdescottes MozReview-Commit-ID: HGINxducS4x
devtools/client/inspector/rules/test/browser.ini
devtools/client/inspector/rules/test/browser_rules_gridline-names-autocomplete.js
devtools/client/inspector/rules/test/doc_grid_names.html
devtools/client/inspector/rules/views/text-property-editor.js
devtools/client/shared/inplace-editor.js
devtools/server/actors/layout.js
devtools/shared/specs/layout.js
--- a/devtools/client/inspector/rules/test/browser.ini
+++ b/devtools/client/inspector/rules/test/browser.ini
@@ -11,16 +11,17 @@ support-files =
   doc_content_stylesheet_script.css
   doc_copystyles.css
   doc_copystyles.html
   doc_cssom.html
   doc_custom.html
   doc_edit_imported_selector.html
   doc_filter.html
   doc_frame_script.js
+  doc_grid_names.html
   doc_inline_sourcemap.html
   doc_invalid_sourcemap.css
   doc_invalid_sourcemap.html
   doc_keyframeanimation.css
   doc_keyframeanimation.html
   doc_keyframeLineNumbers.html
   doc_media_queries.html
   doc_pseudoelement.html
@@ -171,16 +172,17 @@ skip-if = (os == "win" && debug) # bug 9
 [browser_rules_grid-highlighter-on-navigate.js]
 [browser_rules_grid-highlighter-on-reload.js]
 [browser_rules_grid-highlighter-restored-after-reload.js]
 [browser_rules_grid-toggle_01.js]
 [browser_rules_grid-toggle_01b.js]
 [browser_rules_grid-toggle_02.js]
 [browser_rules_grid-toggle_03.js]
 [browser_rules_grid-toggle_04.js]
+[browser_rules_gridline-names-autocomplete.js]
 [browser_rules_guessIndentation.js]
 [browser_rules_highlight-used-fonts.js]
 [browser_rules_inherited-properties_01.js]
 [browser_rules_inherited-properties_02.js]
 [browser_rules_inherited-properties_03.js]
 [browser_rules_inherited-properties_04.js]
 [browser_rules_inline-source-map.js]
 [browser_rules_invalid.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/browser_rules_gridline-names-autocomplete.js
@@ -0,0 +1,169 @@
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Tests that CSS property values are autocompleted and cycled
+// correctly when editing an existing property in the rule view.
+
+// format :
+//  [
+//    what key to press,
+//    modifers,
+//    expected input box value after keypress,
+//    is the popup open,
+//    is a suggestion selected in the popup,
+//    expect ruleview-changed,
+//  ]
+
+const OPEN = true, SELECTED = true, CHANGE = true;
+const changeTestData = [
+  ["c", {}, "col1-start", OPEN, SELECTED, CHANGE],
+  ["o", {}, "col1-start", OPEN, SELECTED, CHANGE],
+  ["l", {}, "col1-start", OPEN, SELECTED, CHANGE],
+  ["VK_DOWN", {}, "col2-start", OPEN, SELECTED, CHANGE],
+  ["VK_RIGHT", {}, "col2-start", !OPEN, !SELECTED, !CHANGE],
+];
+
+// Creates a new CSS property value.
+// Checks that grid-area autocompletes column and row names.
+const newAreaTestData = [
+  ["g", {}, "grid", OPEN, SELECTED, !CHANGE],
+  ["VK_DOWN", {}, "grid-area", OPEN, SELECTED, !CHANGE],
+  ["VK_TAB", {}, "", !OPEN, !SELECTED, !CHANGE],
+  "grid-line-names-updated",
+  ["c", {}, "col1-start", OPEN, SELECTED, CHANGE],
+  ["VK_BACK_SPACE", {}, "c", !OPEN, !SELECTED, CHANGE],
+  ["VK_BACK_SPACE", {}, "", !OPEN, !SELECTED, CHANGE],
+  ["r", {}, "row1-start", OPEN, SELECTED, CHANGE],
+  ["r", {}, "rr", !OPEN, !SELECTED, CHANGE],
+  ["VK_BACK_SPACE", {}, "r", !OPEN, !SELECTED, CHANGE],
+  ["o", {}, "row1-start", OPEN, SELECTED, CHANGE],
+  ["VK_RETURN", {}, "", !OPEN, !SELECTED, CHANGE],
+];
+
+// Creates a new CSS property value.
+// Checks that grid-row only autocompletes row names.
+const newRowTestData = [
+  ["g", {}, "grid", OPEN, SELECTED, !CHANGE],
+  ["r", {}, "grid", OPEN, SELECTED, !CHANGE],
+  ["i", {}, "grid", OPEN, SELECTED, !CHANGE],
+  ["d", {}, "grid", OPEN, SELECTED, !CHANGE],
+  ["-", {}, "grid-area", OPEN, SELECTED, !CHANGE],
+  ["r", {}, "grid-row", OPEN, SELECTED, !CHANGE],
+  ["VK_RETURN", {}, "", !OPEN, !SELECTED, !CHANGE],
+  "grid-line-names-updated",
+  ["c", {}, "c", !OPEN, !SELECTED, CHANGE],
+  ["VK_BACK_SPACE", {}, "", !OPEN, !SELECTED, CHANGE],
+  ["r", {}, "row1-start", OPEN, SELECTED, CHANGE],
+  ["VK_TAB", {}, "", !OPEN, !SELECTED, CHANGE],
+];
+
+const TEST_URL = URL_ROOT + "doc_grid_names.html";
+
+add_task(async function() {
+  await addTab(TEST_URL);
+  let {toolbox, inspector, view} = await openRuleView();
+
+  info("Test autocompletion changing a preexisting property");
+  await runChangePropertyAutocompletionTest(toolbox, inspector, view, changeTestData);
+
+  info("Test autocompletion creating a new property");
+  await runNewPropertyAutocompletionTest(toolbox, inspector, view, newAreaTestData);
+
+  info("Test autocompletion creating a new property");
+  await runNewPropertyAutocompletionTest(toolbox, inspector, view, newRowTestData);
+});
+
+async function runNewPropertyAutocompletionTest(toolbox, inspector, view, testData) {
+  info("Selecting the test node");
+  await selectNode("#cell2", inspector);
+
+  info("Focusing the css property editable field");
+  let ruleEditor = getRuleViewRuleEditor(view, 0);
+  let editor = await focusNewRuleViewProperty(ruleEditor);
+  let gridLineNamesUpdated = inspector.once("grid-line-names-updated");
+
+  info("Starting to test for css property completion");
+  for (let data of testData) {
+    if (data == "grid-line-names-updated") {
+      await gridLineNamesUpdated;
+      continue;
+    }
+    await testCompletion(data, editor, view);
+  }
+}
+
+async function runChangePropertyAutocompletionTest(toolbox, inspector, view, testData) {
+  info("Selecting the test node");
+  await selectNode("#cell3", inspector);
+
+  let ruleEditor = getRuleViewRuleEditor(view, 1).rule;
+  let prop = ruleEditor.textProps[0];
+
+  info("Focusing the css property editable value");
+  let gridLineNamesUpdated = inspector.once("grid-line-names-updated");
+  let editor = await focusEditableField(view, prop.editor.valueSpan);
+  await gridLineNamesUpdated;
+
+  info("Starting to test for css property completion");
+  for (let data of testData) {
+    // Re-define the editor at each iteration, because the focus may have moved
+    // from property to value and back
+    editor = inplaceEditor(view.styleDocument.activeElement);
+    await testCompletion(data, editor, view);
+  }
+}
+
+async function testCompletion([key, modifiers, completion, open, selected, change],
+                         editor, view) {
+  info("Pressing key " + key);
+  info("Expecting " + completion);
+  info("Is popup opened: " + open);
+  info("Is item selected: " + selected);
+
+  let onDone;
+  if (change) {
+    // If the key triggers a ruleview-changed, wait for that event, it will
+    // always be the last to be triggered and tells us when the preview has
+    // been done.
+    onDone = view.once("ruleview-changed");
+  } else {
+    // Otherwise, expect an after-suggest event (except if the popup gets
+    // closed).
+    onDone = key !== "VK_RIGHT" && key !== "VK_BACK_SPACE"
+             ? editor.once("after-suggest")
+             : null;
+  }
+
+  // Also listening for popup opened/closed events if needed.
+  let popupEvent = open ? "popup-opened" : "popup-closed";
+  let onPopupEvent = editor.popup.isOpen !== open ? once(editor.popup, popupEvent) : null;
+
+  info("Synthesizing key " + key + ", modifiers: " + Object.keys(modifiers));
+
+  EventUtils.synthesizeKey(key, modifiers, view.styleWindow);
+
+  // Flush the debounce for the preview text.
+  view.debounce.flush();
+
+  await onDone;
+  await onPopupEvent;
+
+  // The key might have been a TAB or shift-TAB, in which case the editor will
+  // be a new one
+  editor = inplaceEditor(view.styleDocument.activeElement);
+
+  info("Checking the state");
+  if (completion !== null) {
+    is(editor.input.value, completion, "Correct value is autocompleted");
+  }
+
+  if (!open) {
+    ok(!(editor.popup && editor.popup.isOpen), "Popup is closed");
+  } else {
+    ok(editor.popup.isOpen, "Popup is open");
+    is(editor.popup.selectedIndex !== -1, selected, "An item is selected");
+  }
+}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/rules/test/doc_grid_names.html
@@ -0,0 +1,17 @@
+<!doctype html>
+<style type='text/css'>
+  #grid {
+    display: grid;
+    grid-template-rows: [row1-start] auto [row2-start] auto [row2-end];
+    grid-template-columns: [col1-start] 100px [col2-start] 100px [col3-start] 100px [col3-end];
+  }
+  #cell3 {
+    grid-column: "col3-start";
+    grid-row: "row2-start";
+  }
+</style>
+<div id="grid">
+  <div>cell1</div>
+  <div id="cell2">cell2</div>
+  <div id="cell3">cell3</div>
+</div>
\ No newline at end of file
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -90,16 +90,17 @@ function TextPropertyEditor(ruleEditor, 
   this._onNameDone = this._onNameDone.bind(this);
   this._onValueDone = this._onValueDone.bind(this);
   this._onSwatchCommit = this._onSwatchCommit.bind(this);
   this._onSwatchPreview = this._onSwatchPreview.bind(this);
   this._onSwatchRevert = this._onSwatchRevert.bind(this);
   this._onValidate = this.ruleView.debounce(this._previewValue, 10, this);
   this.update = this.update.bind(this);
   this.updatePropertyState = this.updatePropertyState.bind(this);
+  this.getGridlineNames = this.getGridlineNames.bind(this);
 
   this._create();
   this.update();
 }
 
 TextPropertyEditor.prototype = {
   /**
    * Boolean indicating if the name or value is being currently edited.
@@ -311,21 +312,50 @@ TextPropertyEditor.prototype = {
         contentType: InplaceEditor.CONTENT_TYPES.CSS_VALUE,
         property: this.prop,
         defaultIncrement: this.prop.name === "opacity" ? 0.1 : 1,
         popup: this.popup,
         multiline: true,
         maxWidth: () => this.container.getBoundingClientRect().width,
         cssProperties: this.cssProperties,
         cssVariables: this.rule.elementStyle.variables,
+        getGridLineNames: this.getGridlineNames,
       });
     }
   },
 
   /**
+   * Get the grid line names of the grid that the currently selected element is
+   * contained in.
+   *
+   * @return {Object} Contains the names of the cols and rows as arrays
+   * {cols: [], rows: []}.
+   */
+  getGridlineNames: async function() {
+    let gridLineNames = {cols: [], rows: []};
+    let layoutInspector = await this.ruleView.inspector.walker.getLayoutInspector();
+    let gridFront = await layoutInspector.getCurrentGrid(
+      this.ruleView.inspector.selection.nodeFront);
+    if (gridFront) {
+      let gridFragments = gridFront.gridFragments;
+      for (let gridFragment of gridFragments) {
+        for (let rowLine of gridFragment.rows.lines) {
+          gridLineNames.rows = gridLineNames.rows.concat(rowLine.names);
+        }
+        for (let colLine of gridFragment.cols.lines) {
+          gridLineNames.cols = gridLineNames.cols.concat(colLine.names);
+        }
+      }
+    }
+    // Emit message for test files
+    this.ruleView.inspector.emit("grid-line-names-updated");
+    return gridLineNames;
+  },
+
+  /**
    * Get the path from which to resolve requests for this
    * rule's stylesheet.
    *
    * @return {String} the stylesheet's href.
    */
   get sheetHref() {
     let domRule = this.rule.domRule;
     if (domRule) {
--- a/devtools/client/shared/inplace-editor.js
+++ b/devtools/client/shared/inplace-editor.js
@@ -46,16 +46,24 @@ const MAX_POPUP_ENTRIES = 500;
 const FOCUS_FORWARD = focusManager.MOVEFOCUS_FORWARD;
 const FOCUS_BACKWARD = focusManager.MOVEFOCUS_BACKWARD;
 
 const WORD_REGEXP = /\w/;
 const isWordChar = function(str) {
   return str && WORD_REGEXP.test(str);
 };
 
+const GRID_PROPERTY_NAMES = ["grid-area", "grid-row", "grid-row-start",
+                             "grid-row-end", "grid-column", "grid-column-start",
+                             "grid-column-end"];
+const GRID_ROW_PROPERTY_NAMES = ["grid-area", "grid-row", "grid-row-start",
+                                 "grid-row-end"];
+const GRID_COL_PROPERTY_NAMES = ["grid-area", "grid-column", "grid-column-start",
+                                 "grid-column-end"];
+
 /**
  * Helper to check if the provided key matches one of the expected keys.
  * Keys will be prefixed with DOM_VK_ and should match a key in KeyCodes.
  *
  * @param {String} key
  *        the key to check (can be a keyCode).
  * @param {...String} keys
  *        list of possible keys allowed.
@@ -126,16 +134,19 @@ function isKeyIn(key, ...keys) {
  *      defaults to true
  *    {Boolean} preserveTextStyles: If true, do not copy text-related styles
  *              from `element` to the new input.
  *      defaults to false
  *    {Object} cssProperties: An instance of CSSProperties.
  *    {Object} cssVariables: A Map object containing all CSS variables.
  *    {Number} defaultIncrement: The value by which the input is incremented
  *      or decremented by default (0.1 for properties like opacity and 1 by default)
+ *    {Function} getGridLineNames:
+ *       Will be called before offering autocomplete sugestions, if the property is
+ *       a member of GRID_PROPERTY_NAMES.
  */
 function editableField(options) {
   return editableItem(options, function(element, event) {
     if (!options.element.inplaceEditor) {
       new InplaceEditor(options, event);
     }
   });
 }
@@ -290,20 +301,16 @@ function InplaceEditor(options, event) {
   }
 
   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);
-  }
-
   this.input.addEventListener("blur", this._onBlur);
   this.input.addEventListener("keypress", this._onKeyPress);
   this.input.addEventListener("input", this._onInput);
   this.input.addEventListener("dblclick", this._stopEventPropagation);
   this.input.addEventListener("click", this._stopEventPropagation);
   this.input.addEventListener("mousedown", this._stopEventPropagation);
   this.input.addEventListener("contextmenu", this._onContextMenu);
   this.doc.defaultView.addEventListener("blur", this._onWindowBlur);
@@ -316,16 +323,18 @@ function InplaceEditor(options, event) {
 
   this._updateSize();
 
   EventEmitter.decorate(this);
 
   if (options.start) {
     options.start(this, event);
   }
+
+  this._getGridNamesBeforeCompletion(options.getGridLineNames);
 }
 
 exports.InplaceEditor = InplaceEditor;
 
 InplaceEditor.CONTENT_TYPES = CONTENT_TYPES;
 
 InplaceEditor.prototype = {
 
@@ -988,16 +997,35 @@ InplaceEditor.prototype = {
       this._acceptPopupSuggestion();
     } else {
       this._apply();
       this._clear();
     }
   },
 
   /**
+   * Before offering autocomplete, set this.gridLineNames as the line names
+   * of the current grid, if they exist.
+   *
+   * @param {Function} getGridLineNames
+   *        A function which gets the line names of the current grid.
+   */
+  _getGridNamesBeforeCompletion: async function(getGridLineNames) {
+    if (getGridLineNames && this.property &&
+        GRID_PROPERTY_NAMES.includes(this.property.name)) {
+      this.gridLineNames = await getGridLineNames();
+    }
+
+    if (this.contentType == CONTENT_TYPES.CSS_VALUE && this.input &&
+        this.input.value == "") {
+      this._maybeSuggestCompletion(false);
+    }
+  },
+
+  /**
    * Event handler called by the autocomplete popup when receiving a click
    * event.
    */
   _onAutocompletePopupClick: function() {
     this._acceptPopupSuggestion();
   },
 
   _acceptPopupSuggestion: function() {
@@ -1562,17 +1590,28 @@ InplaceEditor.prototype = {
    * Returns a list of CSS values valid for a provided property name to use for
    * the autocompletion. This method is overridden by tests in order to use
    * mocked suggestion lists.
    *
    * @param {String} propertyName
    * @return {Array} array of CSS property values (Strings)
    */
   _getCSSValuesForPropertyName: function(propertyName) {
-    return this.cssProperties.getValues(propertyName);
+    let gridLineList = [];
+    if (this.gridLineNames) {
+      if (GRID_ROW_PROPERTY_NAMES.includes(this.property.name)) {
+        gridLineList.push(...this.gridLineNames.rows);
+      }
+      if (GRID_COL_PROPERTY_NAMES.includes(this.property.name)) {
+        gridLineList.push(...this.gridLineNames.cols);
+      }
+    }
+    // Must be alphabetically sorted before comparing the results with
+    // the user input, otherwise we will lose some results.
+    return gridLineList.concat(this.cssProperties.getValues(propertyName)).sort();
   },
 
   /**
    * Returns the list of all CSS variables to use for the autocompletion.
    *
    * @return {Array} array of CSS variable names (Strings)
    */
   _getCSSVariableNames: function() {
--- a/devtools/server/actors/layout.js
+++ b/devtools/server/actors/layout.js
@@ -137,27 +137,30 @@ const LayoutActor = ActorClassWithSpec(l
   destroy() {
     Actor.prototype.destroy.call(this);
 
     this.tabActor = null;
     this.walker = null;
   },
 
   /**
-   * Returns the flex container found by iterating on the given selected node. The current
-   * node can be a flex container or flex item. If it is a flex item, returns the parent
-   * flex container. Otherwise, return null if the current or parent node is not a flex
-   * container.
+   * Helper function for getCurrentGrid and getCurrentFlexbox. Returns the grid or
+   * flex container (whichever is requested) found by iterating on the given selected
+   * node. The current node can be a grid/flex container or grid/flex item. If it is a
+   * grid/flex item, returns the parent grid/flex container. Otherwise, returns null
+   * if the current or parent node is not a grid/flex container.
    *
    * @param  {Node|NodeActor} node
    *         The node to start iterating at.
-   * @return {FlexboxActor|Null} The FlexboxActor of the flex container of the give node.
-   * Otherwise, returns null.
+   * @param {String} type
+   *         Can be "grid" or "flex", the display type we are searching for.
+   * @return {GridActor|FlexboxActor|Null} The GridActor or FlexboxActor of the
+   * grid/flex container of the give node. Otherwise, returns null.
    */
-  getCurrentFlexbox(node) {
+  getCurrentDisplay(node, type) {
     if (isNodeDead(node)) {
       return null;
     }
 
     // Given node can either be a Node or a NodeActor.
     if (node.rawNode) {
       node = node.rawNode;
     }
@@ -166,45 +169,80 @@ const LayoutActor = ActorClassWithSpec(l
       nodeFilterConstants.SHOW_ELEMENT);
     let currentNode = treeWalker.currentNode;
     let displayType = this.walker.getNode(currentNode).displayType;
 
     if (!displayType) {
       return null;
     }
 
-    // Check if the current node is a flex container.
-    if (displayType == "inline-flex" || displayType == "flex") {
-      return new FlexboxActor(this, treeWalker.currentNode);
+    if (type == "flex" &&
+        (displayType == "inline-flex" || displayType == "flex")) {
+      return new FlexboxActor(this, currentNode);
+    } else if (type == "grid" &&
+               (displayType == "inline-grid" || displayType == "grid")) {
+      return new GridActor(this, currentNode);
     }
 
     // Otherwise, check if this is a flex item or the parent node is a flex container.
     while ((currentNode = treeWalker.parentNode())) {
       if (!currentNode) {
         break;
       }
 
       displayType = this.walker.getNode(currentNode).displayType;
 
-      switch (displayType) {
-        case "inline-flex":
-        case "flex":
-          return new FlexboxActor(this, currentNode);
-        case "contents":
-          // Continue walking up the tree since the parent node is a content element.
-          continue;
+      if (type == "flex" &&
+          (displayType == "inline-flex" || displayType == "flex")) {
+        return new FlexboxActor(this, currentNode);
+      } else if (type == "grid" &&
+                 (displayType == "inline-grid" || displayType == "grid")) {
+        return new GridActor(this, currentNode);
+      } else if (displayType == "contents") {
+        // Continue walking up the tree since the parent node is a content element.
+        continue;
       }
 
       break;
     }
 
     return null;
   },
 
   /**
+   * Returns the grid container found by iterating on the given selected node. The current
+   * node can be a grid container or grid item. If it is a grid item, returns the parent
+   * grid container. Otherwise, return null if the current or parent node is not a grid
+   * container.
+   *
+   * @param  {Node|NodeActor} node
+   *         The node to start iterating at.
+   * @return {GridActor|Null} The GridActor of the grid container of the give node.
+   * Otherwise, returns null.
+   */
+  getCurrentGrid(node) {
+    return this.getCurrentDisplay(node, "grid");
+  },
+
+  /**
+   * Returns the flex container found by iterating on the given selected node. The current
+   * node can be a flex container or flex item. If it is a flex item, returns the parent
+   * flex container. Otherwise, return null if the current or parent node is not a flex
+   * container.
+   *
+   * @param  {Node|NodeActor} node
+   *         The node to start iterating at.
+   * @return {FlexboxActor|Null} The FlexboxActor of the flex container of the give node.
+   * Otherwise, returns null.
+   */
+  getCurrentFlexbox(node) {
+    return this.getCurrentDisplay(node, "flex");
+  },
+
+  /**
    * Returns an array of GridActor objects for all the grid elements contained in the
    * given root node.
    *
    * @param  {Node|NodeActor} node
    *         The root node for grid elements
    * @return {Array} An array of GridActor objects.
    */
   getGrids(node) {
--- a/devtools/shared/specs/layout.js
+++ b/devtools/shared/specs/layout.js
@@ -26,16 +26,25 @@ const layoutSpec = generateActorSpec({
       request: {
         node: Arg(0, "domnode"),
       },
       response: {
         flexbox: RetVal("nullable:flexbox")
       }
     },
 
+    getCurrentGrid: {
+      request: {
+        node: Arg(0, "domnode"),
+      },
+      response: {
+        grid: RetVal("nullable:grid")
+      }
+    },
+
     getGrids: {
       request: {
         rootNode: Arg(0, "domnode")
       },
       response: {
         grids: RetVal("array:grid")
       }
     },