Handle user sticky mode toggles during smart sticky mode

Here's another corner case.

Suppose a user toggles (either on or off) sticky mode while over an editable.

We need to ensure that, even if the user continues to move around the editable or any related nodes of the editable, that we don't interfere with sticky mode state.

For example, a user turns on sticky mode while over a content editable. The user navigates by character or word. This would trigger a change in the current range and potentially turn back off sticky mode.

RELNOTES: n/a
Change-Id: I728060fb30beaa0697b7078332201ed1bee45ecf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2142712
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Akihiro Ota <akihiroota@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757848}

chrome/browser/resources/chromeos/accessibility/chromevox/background/command_handler.js

@@ -110,6 +110,8 @@ CommandHandler.onCommand = function(command) {
       } else {
         chrome.accessibilityPrivate.setKeyboardListener(true, false);
       }
+      CommandHandler.smartStickyMode_.onStickyModeCommand(
+          ChromeVoxState.instance.currentRange);
       return false;
     case 'passThroughMode':
       ChromeVox.passThroughMode = true;

chrome/browser/resources/chromeos/accessibility/chromevox/background/smart_sticky_mode.js

@@ -10,6 +10,7 @@
 
 goog.provide('SmartStickyMode');
 
+goog.require('AutomationUtil');
 goog.require('ChromeVoxState');
 
 /** @implements {ChromeVoxStateObserver} */
@@ -24,6 +25,8 @@ SmartStickyMode = class {
      * @private {boolean}
      */
     this.didTurnOffStickyMode_ = false;
+    /** @private {chrome.automation.AutomationNode|undefined} */
+    this.ignoredNodeSubtree_;
 
     ChromeVoxState.addObserver(this);
   }
@@ -35,34 +38,25 @@ SmartStickyMode = class {
       return;
     }
 
-    // Several cases arise which may lead to a sticky mode toggle:
-    // The node is either editable itself or a descendant of an editable.
-    // The node is a relation target of an editable.
     const node = newRange.start.node;
-    let shouldTurnOffStickyMode = false;
-    if (node.state[chrome.automation.StateType.EDITABLE] ||
-        (node.parent &&
-         node.parent.state[chrome.automation.StateType.EDITABLE])) {
-      // This covers both editable nodes, and inline text boxes (which are not
-      // editable themselves, but may have an editable parent).
-      shouldTurnOffStickyMode = true;
-    } else {
-      let focus = node;
-      while (!shouldTurnOffStickyMode && focus) {
-        if (focus.activeDescendantFor && focus.activeDescendantFor.length) {
-          shouldTurnOffStickyMode |= focus.activeDescendantFor.some(
-              (n) => n.state[chrome.automation.StateType.EDITABLE]);
-        }
-
-        if (focus.controlledBy && focus.controlledBy.length) {
-          shouldTurnOffStickyMode |= focus.controlledBy.some(
-              (n) => n.state[chrome.automation.StateType.EDITABLE]);
-        }
-
-        focus = focus.parent;
+    if (this.ignoredNodeSubtree_) {
+      const editableOrRelatedEditable =
+          this.getEditableOrRelatedEditable_(node);
+      if (editableOrRelatedEditable &&
+          AutomationUtil.isDescendantOf(
+              editableOrRelatedEditable, this.ignoredNodeSubtree_)) {
+        return;
       }
+
+      // Clear it when the user leaves the subtree.
+      this.ignoredNodeSubtree_ = undefined;
     }
 
+    // Several cases arise which may lead to a sticky mode toggle:
+    // The node is either editable itself or a descendant of an editable.
+    // The node is a relation target of an editable.
+    const shouldTurnOffStickyMode = !!this.getEditableOrRelatedEditable_(node);
+
     // This toggler should not make any changes when the range isn't what we're
     // lloking for and we haven't previously tracked any sticky mode state from
     // the user.
@@ -110,4 +104,82 @@ SmartStickyMode = class {
   stopIgnoringRangeChanges() {
     this.ignoreRangeChanges_ = false;
   }
+
+  /**
+   * Called whenever a user toggles sticky mode. In this case, we need to ensure
+   * we reset our internal state appropriately.
+   * @param {!cursors.Range} range The range when the sticky mode command was
+   *     received.
+   */
+  onStickyModeCommand(range) {
+    if (!this.didTurnOffStickyMode_) {
+      return;
+    }
+
+    this.didTurnOffStickyMode_ = false;
+
+    // Resetting the above isn't quite enough. We now have to track the current
+    // range, if it is editable or has an editable relation, to ensure we don't
+    // interfere with the user's sticky mode state.
+    if (!range || !range.start) {
+      return;
+    }
+
+    let editable = this.getEditableOrRelatedEditable_(range.start.node);
+    if (!editable) {
+      return;
+    }
+
+    while (!editable.editableRoot) {
+      editable = editable.parent;
+    }
+    this.ignoredNodeSubtree_ = editable;
+  }
+
+  /**
+   * @param {chrome.automation.AutomationNode} node
+   * @return {chrome.automation.AutomationNode}
+   * @private
+   */
+  getEditableOrRelatedEditable_(node) {
+    if (!node) {
+      return null;
+    }
+
+    if (node.state[chrome.automation.StateType.EDITABLE]) {
+      return node;
+    } else if (
+        node.parent &&
+        node.parent.state[chrome.automation.StateType.EDITABLE]) {
+      // This covers inline text boxes (which are not
+      // editable themselves, but may have an editable parent).
+      return node.parent;
+    } else {
+      let focus = node;
+      let found;
+      while (!found && focus) {
+        if (focus.activeDescendantFor && focus.activeDescendantFor.length) {
+          found = focus.activeDescendantFor.find(
+              (n) => n.state[chrome.automation.StateType.EDITABLE]);
+        }
+
+        if (found) {
+          return found;
+        }
+
+        if (focus.controlledBy && focus.controlledBy.length) {
+          found = focus.controlledBy.find(
+              (n) => n.state[chrome.automation.StateType.EDITABLE]);
+        }
+
+        if (found) {
+          return found;
+        }
+
+        focus = focus.parent;
+      }
+    }
+
+    return null;
+  }
 };

chrome/browser/resources/chromeos/accessibility/chromevox/background/smart_sticky_mode_test.js

@@ -10,54 +10,103 @@ GEN_INCLUDE([
 /**
  * Test fixture for SmartStickyMode.
  */
-ChromeVoxSmartStickyModeTest = class extends ChromeVoxNextE2ETest {};
+ChromeVoxSmartStickyModeTest = class extends ChromeVoxNextE2ETest {
+  /** @override */
+  setUp() {
+    this.ssm_ = new SmartStickyMode();
+    // Deregister from actual range changes.
+    ChromeVoxState.removeObserver(this.ssm_);
+    assertFalse(ssm.didTurnOffStickyMode_);
+  }
+
+  assertDidTurnOffForNode(node) {
+    this.ssm_.onCurrentRangeChanged(cursors.Range.fromNode(node));
+    assertTrue(this.ssm_.didTurnOffStickyMode_);
+  }
+
+  assertDidNotTurnOffForNode(node) {
+    this.ssm_.onCurrentRangeChanged(cursors.Range.fromNode(node));
+    assertFalse(this.ssm_.didTurnOffStickyMode_);
+  }
+
+  get relationsDoc() {
+    return `
+      <p>start</p>
+      <input aria-controls="controls-target" type="text"></input>
+      <textarea aria-activedescendant="active-descendant-target"></textarea>
+      <div contenteditable><h3>hello</h3></div>
+      <ul id="controls-target"><li>end</ul>
+      <ul id="active-descendant-target"><li>end</ul>
+    `;
+  }
+};
 
 TEST_F('ChromeVoxSmartStickyModeTest', 'PossibleRangeTypes', function() {
-  this.runWithLoadedTree(
-      `
-    <p>start</p>
-    <input aria-controls="controls-target" type="text"></input>
-    <textarea aria-activedescendant="active-descendant-target"></textarea>
-    <div contenteditable><h3>hello</h3></div>
-    <ul id="controls-target"><li>end</ul>
-    <ul id="active-descendant-target"><li>end</ul>
-  `,
-      function(root) {
-        const ssm = new SmartStickyMode();
-
-        // Deregister from actual range changes.
-        ChromeVoxState.removeObserver(ssm);
-        assertFalse(ssm.didTurnOffStickyMode_);
-        const [p, input, textarea, contenteditable, ul1, ul2] = root.children;
+  this.runWithLoadedTree(this.relationsDoc, function(root) {
+    const [p, input, textarea, contenteditable, ul1, ul2] = root.children;
 
-        function assertDidTurnOffForNode(node) {
-          ssm.onCurrentRangeChanged(cursors.Range.fromNode(node));
-          assertTrue(ssm.didTurnOffStickyMode_);
-        }
+    // First, turn on sticky mode and try changing range to various parts of
+    // the document.
+    ChromeVoxBackground.setPref(
+        'sticky', true /* value */, true /* announce */);
+    this.assertDidTurnOffForNode(input);
+    this.assertDidTurnOffForNode(textarea);
+    this.assertDidNotTurnOffForNode(p);
+    this.assertDidTurnOffForNode(contenteditable);
+    this.assertDidTurnOffForNode(ul1);
+    this.assertDidNotTurnOffForNode(p);
+    this.assertDidTurnOffForNode(ul2);
+    this.assertDidTurnOffForNode(ul1.firstChild);
+    this.assertDidNotTurnOffForNode(ul1.parent);
+    this.assertDidNotTurnOffForNode(ul2.parent);
+    this.assertDidNotTurnOffForNode(p);
+    this.assertDidTurnOffForNode(ul2.firstChild);
+    this.assertDidNotTurnOffForNode(p);
+    this.assertDidNotTurnOffForNode(contenteditable.parent);
+    this.assertDidTurnOffForNode(contenteditable.find({role: 'heading'}));
+    this.assertDidTurnOffForNode(contenteditable.find({role: 'inlineTextBox'}));
+  });
+});
 
-        function assertDidNotTurnOffForNode(node) {
-          ssm.onCurrentRangeChanged(cursors.Range.fromNode(node));
-          assertFalse(ssm.didTurnOffStickyMode_);
-        }
+TEST_F(
+    'ChromeVoxSmartStickyModeTest', 'UserPressesStickyModeCommand', function() {
+      this.runWithLoadedTree(this.relationsDoc, function(root) {
+        const [p, input, textarea, contenteditable, ul1, ul2] = root.children;
+        ChromeVoxBackground.setPref(
+            'sticky', true /* value */, true /* announce */);
+
+        // Mix in calls to turn on / off sticky mode while moving the range
+        // around.
+        this.assertDidTurnOffForNode(input);
+        this.ssm_.onStickyModeCommand(cursors.Range.fromNode(input));
+        this.assertDidNotTurnOffForNode(input);
+        this.ssm_.onStickyModeCommand(cursors.Range.fromNode(input));
+        this.assertDidNotTurnOffForNode(input);
+        this.assertDidNotTurnOffForNode(input.firstChild);
+        this.assertDidNotTurnOffForNode(p);
 
-        // First, turn on sticky mode and try changing range to various parts of
-        // the document.
+        // Make sure sticky mode is on again. This call doesn't impact our
+        // instance of SmartStickyMode.
         ChromeVoxBackground.setPref(
             'sticky', true /* value */, true /* announce */);
 
-        assertDidTurnOffForNode(input);
-        assertDidTurnOffForNode(textarea);
-        assertDidNotTurnOffForNode(p);
-
-        assertDidTurnOffForNode(contenteditable);
-        assertDidTurnOffForNode(ul1);
-        assertDidNotTurnOffForNode(p);
-        assertDidTurnOffForNode(ul2);
-        assertDidTurnOffForNode(ul1.firstChild);
-        assertDidNotTurnOffForNode(p);
-        assertDidTurnOffForNode(ul2.firstChild);
-        assertDidNotTurnOffForNode(p);
-        assertDidTurnOffForNode(contenteditable.find({role: 'heading'}));
-        assertDidTurnOffForNode(contenteditable.find({role: 'inlineTextBox'}));
+        // Mix in more sticky mode user commands and move to related nodes.
+        this.assertDidTurnOffForNode(contenteditable);
+        this.assertDidTurnOffForNode(ul2);
+        this.ssm_.onStickyModeCommand(cursors.Range.fromNode(ul2));
+        this.assertDidNotTurnOffForNode(ul2);
+        this.assertDidNotTurnOffForNode(ul2.firstChild);
+        this.assertDidNotTurnOffForNode(contenteditable);
+        this.ssm_.onStickyModeCommand(cursors.Range.fromNode(input));
+        this.assertDidNotTurnOffForNode(ul2);
+        this.assertDidNotTurnOffForNode(ul2.firstChild);
+        this.assertDidNotTurnOffForNode(contenteditable);
+
+        // Finally, verify sticky mode isn't impacted on non-editables.
+        this.assertDidNotTurnOffForNode(p);
+        this.ssm_.onStickyModeCommand(cursors.Range.fromNode(p));
+        this.assertDidNotTurnOffForNode(p);
+        this.ssm_.onStickyModeCommand(cursors.Range.fromNode(p));
+        this.assertDidNotTurnOffForNode(p);
       });
-});
+    });

extensions/common/api/automation.idl

@@ -1005,6 +1005,9 @@ enum MarkerType {
     // Indicates the font family.
     DOMString fontFamily;
 
+    // Indicates whether this is a root of an editable subtree.
+    boolean editableRoot;
+
     //
     // Walking the tree.
     //

extensions/renderer/resources/automation/automation_node.js

@@ -1240,8 +1240,9 @@ var stringAttributes = [
     'value'];
 
 var boolAttributes = [
-  'busy', 'clickable', 'containerLiveAtomic', 'containerLiveBusy', 'liveAtomic',
-  'modal', 'scrollable', 'selected', 'supportsTextLocation'
+  'busy', 'clickable', 'containerLiveAtomic', 'containerLiveBusy',
+  'editableRoot', 'liveAtomic', 'modal', 'scrollable', 'selected',
+  'supportsTextLocation'
 ];
 
 var intAttributes = [

third_party/closure_compiler/externs/automation.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 // This file was generated by:
-//   tools\json_schema_compiler\compiler.py.
+//   tools/json_schema_compiler/compiler.py.
 // NOTE: The format of types has changed. 'FooType' is now
 //   'chrome.automation.FooType'.
 // Please run the closure compiler before committing changes.
@@ -1440,6 +1440,13 @@ chrome.automation.AutomationNode.prototype.fontSize;
  */
 chrome.automation.AutomationNode.prototype.fontFamily;
 
+/**
+ * Indicates whether this is a root of an editable subtree.
+ * @type {boolean}
+ * @see https://developer.chrome.com/extensions/automation#type-editableRoot
+ */
+chrome.automation.AutomationNode.prototype.editableRoot;
+
 /**
  * Walking the tree.
  * @type {!Array<!chrome.automation.AutomationNode>}