Bug 1166959 - Allow the selector editor to advance the focus with tab, shift-tab and enter r=pbrosset
authorGabriel Luong <gabriel.luong@gmail.com>
Thu, 25 Jun 2015 15:51:52 -0700
changeset 250214 c73a81f1802b8ca4d80784b15b8a7265456122c8
parent 250213 dd1fe44cfbecce75c7b297c07a05af92d2e78145
child 250215 aa1f2a978a64ad6960fc8b2dd21f6fc782f872bb
push id13722
push usergabriel.luong@gmail.com
push dateThu, 25 Jun 2015 22:51:15 +0000
treeherderfx-team@c73a81f1802b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbrosset
bugs1166959
milestone41.0a1
Bug 1166959 - Allow the selector editor to advance the focus with tab, shift-tab and enter r=pbrosset
browser/devtools/shared/inplace-editor.js
browser/devtools/styleinspector/rule-view.js
browser/devtools/styleinspector/test/browser_ruleview_edit-selector-commit.js
--- a/browser/devtools/shared/inplace-editor.js
+++ b/browser/devtools/shared/inplace-editor.js
@@ -229,20 +229,21 @@ function InplaceEditor(aOptions, aEvent)
     this._advanceChars = aCharCode => aCharCode 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(aOptions.selectAll) == "undefined" || aOptions.selectAll) {
     this.input.select();
   }
-  this.input.focus();
 
   if (this.contentType == CONTENT_TYPES.CSS_VALUE && this.input.value == "") {
     this._maybeSuggestCompletion(true);
   }
 
   this.input.addEventListener("blur", this._onBlur, false);
   this.input.addEventListener("keypress", this._onKeyPress, false);
   this.input.addEventListener("input", this._onInput, false);
@@ -954,17 +955,18 @@ InplaceEditor.prototype = {
         if (this.stopOnShiftTab) {
           direction = null;
         } else {
           direction = FOCUS_BACKWARD;
         }
       }
       if ((this.stopOnReturn &&
            aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_RETURN) ||
-          (this.stopOnTab && aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_TAB)) {
+          (this.stopOnTab && aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_TAB &&
+           !aEvent.shiftKey)) {
         direction = null;
       }
 
       // Now we don't want to suggest anything as we are moving out.
       this._preventSuggestions = true;
       // But we still want to show suggestions for css values. i.e. moving out
       // of css property input box in forward direction
       if (this.contentType == CONTENT_TYPES.CSS_PROPERTY &&
--- a/browser/devtools/styleinspector/rule-view.js
+++ b/browser/devtools/styleinspector/rule-view.js
@@ -2705,31 +2705,29 @@ RuleEditor.prototype = {
         title: CssLogic.l10n("rule.selectorHighlighter.tooltip")
       });
       selectorHighlighter.addEventListener("click", () => {
         this.ruleView.toggleSelectorHighlighter(selectorHighlighter, selector);
       });
     }
 
     this.selectorText = createChild(this.selectorContainer, "span", {
-      class: "ruleview-selector theme-fg-color3"
+      class: "ruleview-selector theme-fg-color3",
+      tabindex: this.isSelectorEditable ? "0" : "-1",
     });
 
     if (this.isSelectorEditable) {
       this.selectorContainer.addEventListener("click", aEvent => {
         // Clicks within the selector shouldn't propagate any further.
         aEvent.stopPropagation();
       }, false);
 
       editableField({
         element: this.selectorText,
         done: this._onSelectorDone,
-        stopOnShiftTab: true,
-        stopOnTab: true,
-        stopOnReturn: true
       });
     }
 
     this.openBrace = createChild(header, "span", {
       class: "ruleview-ruleopen",
       textContent: " {"
     });
 
@@ -2996,36 +2994,38 @@ RuleEditor.prototype = {
     }
   },
 
   /**
    * Called when the selector's inplace editor is closed.
    * Ignores the change if the user pressed escape, otherwise
    * commits it.
    *
-   * @param {string} aValue
+   * @param {string} value
    *        The value contained in the editor.
-   * @param {boolean} aCommit
+   * @param {boolean} commit
    *        True if the change should be applied.
+   * @param {number} direction
+   *        The move focus direction number.
    */
-  _onSelectorDone: function(aValue, aCommit) {
-    if (!aCommit || this.isEditing || aValue === "" ||
-        aValue === this.rule.selectorText) {
+  _onSelectorDone: function(value, commit, direction) {
+    if (!commit || this.isEditing || value === "" ||
+        value === this.rule.selectorText) {
       return;
     }
 
     let ruleView = this.ruleView;
     let elementStyle = ruleView._elementStyle;
     let element = elementStyle.element;
     let supportsUnmatchedRules =
       this.rule.domRule.supportsModifySelectorUnmatched;
 
     this.isEditing = true;
 
-    this.rule.domRule.modifySelector(element, aValue).then(response => {
+    this.rule.domRule.modifySelector(element, value).then(response => {
       this.isEditing = false;
 
       if (!supportsUnmatchedRules) {
         if (response) {
           this.ruleView.refreshPanel();
         }
         return;
       }
@@ -3047,20 +3047,44 @@ RuleEditor.prototype = {
       this.element.parentNode.replaceChild(editor.element, this.element);
 
       // Remove highlight for modified selector
       if (ruleView.highlightedSelector &&
           ruleView.highlightedSelector == this.rule.selectorText) {
         ruleView.toggleSelectorHighlighter(ruleView.lastSelectorIcon,
           ruleView.highlightedSelector);
       }
+
+      this._moveSelectorFocus(newRule, direction);
     }).then(null, err => {
       this.isEditing = false;
       promiseWarn(err);
     });
+  },
+
+  /**
+   * Handle moving the focus change after pressing tab and return from the
+   * selector inplace editor. The focused element after a tab or return keypress
+   * is lost because the rule editor is replaced.
+   *
+   * @param {Rule} rule
+   *        The Rule object.
+   * @param {number} direction
+   *        The move focus direction number.
+   */
+  _moveSelectorFocus: function(rule, direction) {
+    if (!direction || direction == Ci.nsIFocusManager.MOVEFOCUS_BACKWARD) {
+      return;
+    }
+
+    if (rule.textProps.length > 0) {
+      rule.textProps[0].editor.nameSpan.click();
+    } else {
+      this.propertyList.click();
+    }
   }
 };
 
 /**
  * Create a TextPropertyEditor.
  *
  * @param {RuleEditor} aRuleEditor
  *        The rule editor that owns this TextPropertyEditor.
--- a/browser/devtools/styleinspector/test/browser_ruleview_edit-selector-commit.js
+++ b/browser/devtools/styleinspector/test/browser_ruleview_edit-selector-commit.js
@@ -2,70 +2,79 @@
 /* Any copyright is dedicated to the Public Domain.
  http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 // Test selector value is correctly displayed when committing the inplace editor
 // with ENTER, ESC, SHIFT+TAB and TAB
 
-let PAGE_CONTENT = [
-  '<style type="text/css">',
-  '  #testid {',
-  '    text-align: center;',
-  '  }',
-  '</style>',
-  '<div id="testid" class="testclass">Styled Node</div>',
+let TEST_URI = [
+  "<style type='text/css'>",
+  "  #testid1 {",
+  "    text-align: center;",
+  "  }",
+  "  #testid2 {",
+  "    text-align: center;",
+  "  }",
+  "  #testid3 {",
+  "  }",
+  "</style>",
+  "<div id='testid1'>Styled Node</div>",
+  "<div id='testid2'>Styled Node</div>",
+  "<div id='testid3'>Styled Node</div>",
 ].join("\n");
 
 const TEST_DATA = [
   {
-    node: "#testid",
+    node: "#testid1",
     value: ".testclass",
     commitKey: "VK_ESCAPE",
     modifiers: {},
-    expected: "#testid"
+    expected: "#testid1",
+
   },
   {
-    node: "#testid",
-    value: ".testclass",
+    node: "#testid1",
+    value: ".testclass1",
     commitKey: "VK_RETURN",
     modifiers: {},
-    expected: ".testclass"
+    expected: ".testclass1"
   },
   {
-    node: "#testid",
-    value: ".testclass",
+    node: "#testid2",
+    value: ".testclass2",
     commitKey: "VK_TAB",
     modifiers: {},
-    expected: ".testclass"
+    expected: ".testclass2"
   },
   {
-    node: "#testid",
-    value: ".testclass",
+    node: "#testid3",
+    value: ".testclass3",
     commitKey: "VK_TAB",
     modifiers: {shiftKey: true},
-    expected: ".testclass"
+    expected: ".testclass3"
   }
 ];
 
 add_task(function*() {
-  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(PAGE_CONTENT));
-  let {toolbox, inspector, view} = yield openRuleView();
+  yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI));
+  let { inspector, view } = yield openRuleView();
 
   info("Iterating over the test data");
   for (let data of TEST_DATA) {
     yield runTestData(inspector, view, data);
   }
 });
 
 function* runTestData(inspector, view, data) {
   let {node, value, commitKey, modifiers, expected} = data;
 
-  info("Updating " + node + " to " + value + " and committing with " + commitKey + ". Expecting: " + expected);
+  info("Updating " + node + " to " + value + " and committing with " +
+       commitKey + ". Expecting: " + expected);
 
   info("Selecting the test element");
   yield selectNode(node, inspector);
 
   let idRuleEditor = getRuleViewRuleEditor(view, 1);
 
   info("Focusing an existing selector name in the rule-view");
   let editor = yield focusEditableField(idRuleEditor.selectorText);
@@ -73,21 +82,37 @@ function* runTestData(inspector, view, d
       "The selector editor got focused");
 
   info("Enter the new selector value: " + value);
   editor.input.value = value;
 
   info("Entering the commit key " + commitKey + " " + modifiers);
   EventUtils.synthesizeKey(commitKey, modifiers);
 
+  let activeElement = view.doc.activeElement;
+
   if (commitKey === "VK_ESCAPE") {
     is(idRuleEditor.rule.selectorText, expected,
         "Value is as expected: " + expected);
-    is(idRuleEditor.isEditing, false, "Selector is not being edited.")
-  } else {
-    yield once(view, "ruleview-changed");
-    ok(getRuleViewRule(view, expected),
-        "Rule with " + name + " selector exists.");
+    is(idRuleEditor.isEditing, false, "Selector is not being edited.");
+    is(idRuleEditor.selectorText, activeElement,
+       "Focus is on selector span.");
+    return;
   }
 
-  info("Resetting page content");
-  content.document.body.innerHTML = PAGE_CONTENT;
+  yield once(view, "ruleview-changed");
+
+  ok(getRuleViewRule(view, expected),
+     "Rule with " + expected + " selector exists.");
+
+  if (modifiers.shiftKey) {
+    idRuleEditor = getRuleViewRuleEditor(view, 0);
+  }
+
+  let rule = idRuleEditor.rule;
+  if (rule.textProps.length > 0) {
+    is(inplaceEditor(rule.textProps[0].editor.nameSpan).input, activeElement,
+       "Focus is on the first property name span.");
+  } else {
+    is(inplaceEditor(idRuleEditor.newPropSpan).input, activeElement,
+       "Focus is on the new property span.");
+  }
 }