Bug 1166959 - Allow the selector editor to advance the focus with tab, shift-tab and enter r=pbrosset

browser/devtools/shared/inplace-editor.js

@@ -234,10 +234,11 @@ function InplaceEditor(aOptions, aEvent)
   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);
@@ -959,7 +960,8 @@ InplaceEditor.prototype = {
       }
       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;
       }
 

browser/devtools/styleinspector/rule-view.js

@@ -2710,7 +2710,8 @@ RuleEditor.prototype = {
     }
 
     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) {
@@ -2722,9 +2723,6 @@ RuleEditor.prototype = {
       editableField({
         element: this.selectorText,
         done: this._onSelectorDone,
-        stopOnShiftTab: true,
-        stopOnTab: true,
-        stopOnReturn: true
       });
     }
 
@@ -3001,14 +2999,16 @@ RuleEditor.prototype = {
    * 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;
     }
 
@@ -3020,7 +3020,7 @@ RuleEditor.prototype = {
 
     this.isEditing = true;
 
-    this.rule.domRule.modifySelector(element, aValue).then(response => {
+    this.rule.domRule.modifySelector(element, value).then(response => {
       this.isEditing = false;
 
       if (!supportsUnmatchedRules) {
@@ -3052,10 +3052,34 @@ RuleEditor.prototype = {
         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();
+    }
   }
 };
 

browser/devtools/styleinspector/test/browser_ruleview_edit-selector-commit.js

@@ -7,49 +7,57 @@
 // 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) {
@@ -60,7 +68,8 @@ add_task(function*() {
 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);
@@ -78,16 +87,32 @@ function* runTestData(inspector, view, data) {
   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.");
+  }
 }