Bug 1876694 - [devtools] Add preference to make Rules view enter key move focus to next input. r=devtools-reviewers,ochameau.

Differential Revision: https://phabricator.services.mozilla.com/D199697

browser/app/profile/firefox.js

@@ -2565,6 +2565,9 @@ pref("devtools.gridinspector.maxHighlighters", 3);
 pref("devtools.inspector.simple-highlighters-reduced-motion", false);
 // Display notice about Enter key behavior in Rules view.
 pref("devtools.inspector.showRulesViewEnterKeyNotice", true);
+// Wheter or not Enter on inplace editor in the Rules view moves focus and activates
+// next inplace editor.
+pref("devtools.inspector.rule-view.focusNextOnEnter", false);
 
 // Whether or not the box model panel is opened in the layout view
 pref("devtools.layout.boxmodel.opened", true);

devtools/client/inspector/rules/rules.js

@@ -67,6 +67,8 @@ const HTML_NS = "http://www.w3.org/1999/xhtml";
 const PREF_UA_STYLES = "devtools.inspector.showUserAgentStyles";
 const PREF_DEFAULT_COLOR_UNIT = "devtools.defaultColorUnit";
 const PREF_DRAGGABLE = "devtools.inspector.draggable_properties";
+const PREF_INPLACE_EDITOR_FOCUS_NEXT_ON_ENTER =
+  "devtools.inspector.rule-view.focusNextOnEnter";
 const FILTER_CHANGED_TIMEOUT = 150;
 // Removes the flash-out class from an element after 1 second.
 const PROPERTY_FLASHING_DURATION = 1000;
@@ -240,6 +242,8 @@ function CssRuleView(inspector, document, store) {
   this._handleDefaultColorUnitPrefChange =
     this._handleDefaultColorUnitPrefChange.bind(this);
   this._handleDraggablePrefChange = this._handleDraggablePrefChange.bind(this);
+  this._handleInplaceEditorFocusNextOnEnterPrefChange =
+    this._handleInplaceEditorFocusNextOnEnterPrefChange.bind(this);
 
   this._prefObserver = new PrefObserver("devtools.");
   this._prefObserver.on(PREF_UA_STYLES, this._handleUAStylePrefChange);
@@ -251,6 +255,13 @@ function CssRuleView(inspector, document, store) {
   // Initialize value of this.draggablePropertiesEnabled
   this._handleDraggablePrefChange();
 
+  this._prefObserver.on(
+    PREF_INPLACE_EDITOR_FOCUS_NEXT_ON_ENTER,
+    this._handleInplaceEditorFocusNextOnEnterPrefChange
+  );
+  // Initialize value of this.inplaceEditorFocusNextOnEnter
+  this._handleInplaceEditorFocusNextOnEnterPrefChange();
+
   this.pseudoClassCheckboxes = this._createPseudoClassCheckboxes();
   this.showUserAgentStyles = Services.prefs.getBoolPref(PREF_UA_STYLES);
 
@@ -683,10 +694,14 @@ CssRuleView.prototype = {
   maybeShowEnterKeyNotice() {
     const SHOW_RULES_VIEW_ENTER_KEY_NOTICE_PREF =
       "devtools.inspector.showRulesViewEnterKeyNotice";
-    // Make the Enter key notice visible
-    // if it wasn't dismissed by the user yet.
+    // Make the Enter key notice visible if it wasn't dismissed by the user yet and they
+    // didn't set devtools.inspector.rule-view.focusNextOnEnter to true.
     if (
-      !Services.prefs.getBoolPref(SHOW_RULES_VIEW_ENTER_KEY_NOTICE_PREF, false)
+      !Services.prefs.getBoolPref(
+        SHOW_RULES_VIEW_ENTER_KEY_NOTICE_PREF,
+        false
+      ) ||
+      this.inplaceEditorFocusNextOnEnter === true
     ) {
       return;
     }
@@ -764,10 +779,22 @@ CssRuleView.prototype = {
     this.emit("draggable-preference-updated");
   },
 
+  _handleInplaceEditorFocusNextOnEnterPrefChange() {
+    this.inplaceEditorFocusNextOnEnter = Services.prefs.getBoolPref(
+      PREF_INPLACE_EDITOR_FOCUS_NEXT_ON_ENTER,
+      false
+    );
+    this._handlePrefChange(PREF_INPLACE_EDITOR_FOCUS_NEXT_ON_ENTER);
+  },
+
   _handlePrefChange(pref) {
     // Reselect the currently selected element
-    const refreshOnPrefs = [PREF_UA_STYLES, PREF_DEFAULT_COLOR_UNIT];
-    if (refreshOnPrefs.indexOf(pref) > -1) {
+    const refreshOnPrefs = [
+      PREF_UA_STYLES,
+      PREF_DEFAULT_COLOR_UNIT,
+      PREF_INPLACE_EDITOR_FOCUS_NEXT_ON_ENTER,
+    ];
+    if (this._viewedElement && refreshOnPrefs.includes(pref)) {
       this.selectElement(this._viewedElement, true);
     }
   },
@@ -885,6 +912,10 @@ CssRuleView.prototype = {
       this._handleDefaultColorUnitPrefChange
     );
     this._prefObserver.off(PREF_DRAGGABLE, this._handleDraggablePrefChange);
+    this._prefObserver.off(
+      PREF_INPLACE_EDITOR_FOCUS_NEXT_ON_ENTER,
+      this._handleInplaceEditorFocusNextOnEnterPrefChange
+    );
     this._prefObserver.destroy();
 
     this._outputParser = null;

devtools/client/inspector/rules/test/browser_rules_keybindings.js

@@ -258,6 +258,66 @@ add_task(async function testKeyboardNavigationInElementRule() {
   );
 });
 
+// Test keyboard navigation in the rule view when
+// devtools.inspector.rule-view.focusNextOnEnter is set to true
+
+add_task(async function () {
+  await pushPref("devtools.inspector.showRulesViewEnterKeyNotice", true);
+  await pushPref("devtools.inspector.rule-view.focusNextOnEnter", true);
+  await addTab(`data:text/html;charset=utf-8,
+    <style>h1 {}</style>
+    <h1>Some header text</h1>`);
+  const { inspector, view } = await openRuleView();
+  await selectNode("h1", inspector);
+
+  const kbdNoticeEl = view.styleDocument.getElementById(
+    "ruleview-kbd-enter-notice"
+  );
+  ok(kbdNoticeEl.hasAttribute("hidden"), "Notice is not displayed by default");
+
+  info("Getting the ruleclose brace element for the `h1` rule");
+  const brace = view.styleDocument.querySelectorAll(".ruleview-ruleclose")[1];
+
+  info("Focus the new property editable field to create a color property");
+  const ruleEditor = getRuleViewRuleEditor(view, 1);
+  await focusNewRuleViewProperty(ruleEditor);
+  EventUtils.sendString("color");
+
+  info("Typing ENTER to focus the next field: property value");
+  let onFocus = once(brace.parentNode, "focus", true);
+  let onRuleViewChanged = view.once("ruleview-changed");
+
+  EventUtils.sendKey("Return");
+
+  await onFocus;
+  await onRuleViewChanged;
+  ok(true, "The value field was focused");
+
+  info("Entering a property value");
+  EventUtils.sendString("tomato");
+
+  info("Typing Enter again should focus a new property name");
+  onFocus = once(brace.parentNode, "focus", true);
+  onRuleViewChanged = view.once("ruleview-changed");
+  EventUtils.sendKey("Return");
+  await onFocus;
+  await onRuleViewChanged;
+
+  const activeElement = view.styleDocument.activeElement;
+  is(
+    `${activeElement.tagName}${[...activeElement.classList]
+      .map(cls => `.${cls}`)
+      .join("")}`,
+    "input.styleinspector-propertyeditor",
+    "The new property name field was focused"
+  );
+
+  ok(
+    kbdNoticeEl.hasAttribute("hidden"),
+    "Notice isn't displayed after hitting Enter"
+  );
+});
+
 function waitForStyleAttributeMutation(view, expectedAttributeValue) {
   return new Promise(r => {
     view.inspector.walker.on(

devtools/client/inspector/rules/views/rule-editor.js

@@ -335,8 +335,9 @@ RuleEditor.prototype = {
         focusEditableFieldContainerSelector: ".ruleview-rule",
         // We don't want Enter to trigger the next editable field, just to validate
         // what the user entered, close the editor, and focus the span so the user can
-        // navigate with the keyboard as expected.
-        stopOnReturn: true,
+        // navigate with the keyboard as expected, unless the user has
+        // devtools.inspector.rule-view.focusNextOnEnter set to true
+        stopOnReturn: this.ruleView.inplaceEditorFocusNextOnEnter !== true,
       });
     }
 

devtools/client/inspector/rules/views/text-property-editor.js

@@ -335,8 +335,9 @@ TextPropertyEditor.prototype = {
         focusEditableFieldContainerSelector: ".ruleview-rule",
         // We don't want Enter to trigger the next editable field, just to validate
         // what the user entered, close the editor, and focus the span so the user can
-        // navigate with the keyboard as expected.
-        stopOnReturn: true,
+        // navigate with the keyboard as expected, unless the user has
+        // devtools.inspector.rule-view.focusNextOnEnter set to true
+        stopOnReturn: this.ruleView.inplaceEditorFocusNextOnEnter !== true,
         inputAriaLabel: PROPERTY_NAME_INPUT_LABEL,
       });
 
@@ -438,8 +439,9 @@ TextPropertyEditor.prototype = {
         focusEditableFieldContainerSelector: ".ruleview-rule",
         // We don't want Enter to trigger the next editable field, just to validate
         // what the user entered, close the editor, and focus the span so the user can
-        // navigate with the keyboard as expected.
-        stopOnReturn: true,
+        // navigate with the keyboard as expected, unless the user has
+        // devtools.inspector.rule-view.focusNextOnEnter set to true
+        stopOnReturn: this.ruleView.inplaceEditorFocusNextOnEnter !== true,
         // Label the value input with the name span so screenreader users know what this
         // applies to.
         inputAriaLabelledBy: this.nameSpan.id,