Make ChromeVox Next a setting in options page.

chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js

Index: chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
diff --git a/chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js b/chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
index 5c5b4dfcd0a3c978021d6a122f9aa5eaa68e0f03..68eaa54b52b7e8a1cbd6de15cffe36c9bc3b84ec 100644
--- a/chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
+++ b/chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
@@ -146,6 +146,11 @@ Background = function() {
     chrome.accessibilityPrivate.onAccessibilityGesture.addListener(
         this.onAccessibilityGesture_);
 
+    if (localStorage['mode']) {
+      var mode = localStorage['mode'];
+      this.setMode(mode);
+    }
+
     Notifications.onStartup();
   }};
 
@@ -200,20 +205,33 @@ Background.prototype = {
   },
 
   /**
+   * This method sets the global mode for ChromeVox.
+   * The following transitions are supported:
+   * classic to force next
+   * compat to force next
+   * force next to classic
+   * force next to compat
    * @override
    */
   setMode: function(mode, opt_injectClassic) {
+    var newMode = mode;
+    var oldMode = this.mode_;
+    this.mode_ = newMode;
+    localStorage['mode'] = mode;

[dmazzoni, 2016/06/20 19:49:03]

Do you want to be setting the mode to things like 'compat'?
It might be harmless, but it's not clear it has any value.

[David Tseng, 2016/06/20 22:31:08]

Now only storing the "nextness" state.

+
     // Switching key maps potentially affects the key codes that involve
     // sequencing. Without resetting this list, potentially stale key codes
     // remain. The key codes themselves get pushed in
     // cvox.KeySequence.deserialize which gets called by cvox.KeyMap.
     cvox.ChromeVox.sequenceSwitchKeyCodes = [];
-    if (mode === ChromeVoxMode.CLASSIC || mode === ChromeVoxMode.COMPAT)
-      window['prefs'].switchToKeyMap('keymap_classic');
-    else
-      window['prefs'].switchToKeyMap('keymap_next');
+    if (window['prefs']) {
+      if (newMode === ChromeVoxMode.CLASSIC || mode === ChromeVoxMode.COMPAT)
+        window['prefs'].switchToKeyMap('keymap_classic');
+      else
+        window['prefs'].switchToKeyMap('keymap_next');
+    }
 
-    if (mode == ChromeVoxMode.CLASSIC) {
+    if (newMode == ChromeVoxMode.CLASSIC) {
       if (chrome.commands &&
           chrome.commands.onCommand.hasListener(this.onGotCommand))
         chrome.commands.onCommand.removeListener(this.onGotCommand);
@@ -231,7 +249,7 @@ Background.prototype = {
     // in time.
     var cur = this.currentRange_;
     chrome.tabs.query({active: true}, function(tabs) {
-      if (mode === ChromeVoxMode.CLASSIC) {
+      if (newMode === ChromeVoxMode.CLASSIC) {
         // Generally, we don't want to inject classic content scripts as it is
         // done by the extension system at document load. The exception is when
         // we toggle classic on manually as part of a user command.
@@ -249,14 +267,14 @@ Background.prototype = {
 
     // If switching out of a ChromeVox Next mode, make sure we cancel
     // the progress loading sound just in case.
-    if ((this.mode_ === ChromeVoxMode.NEXT ||
-         this.mode_ === ChromeVoxMode.FORCE_NEXT) &&
-        this.mode_ != mode) {
+    if ((oldMode === ChromeVoxMode.NEXT ||
+         oldMode === ChromeVoxMode.FORCE_NEXT) &&
+        oldMode != mode) {
       cvox.ChromeVox.earcons.cancelEarcon(cvox.Earcon.PAGE_START_LOADING);
     }
 
-    if (mode === ChromeVoxMode.NEXT ||
-        mode === ChromeVoxMode.FORCE_NEXT) {
+    if (newMode === ChromeVoxMode.NEXT ||
+        newMode === ChromeVoxMode.FORCE_NEXT) {
       (new PanelCommand(PanelCommandType.ENABLE_MENUS)).send();
       if (cvox.TabsApiHandler)
         cvox.TabsApiHandler.shouldOutputSpeechAndBraille = false;
@@ -268,51 +286,92 @@ Background.prototype = {
 
     // If switching to Classic from any automation-API-based mode,
     // clear the focus ring.
-    if (mode === ChromeVoxMode.CLASSIC && mode != this.mode_) {
+    if (newMode === ChromeVoxMode.CLASSIC && newMode != oldMode) {
       if (cvox.ChromeVox.isChromeOS)
         chrome.accessibilityPrivate.setFocusRing([]);
     }
 
     // If switching away from Classic to any automation-API-based mode,
     // update the range based on what's focused.
-    if (this.mode_ === ChromeVoxMode.CLASSIC && mode != this.mode_) {
+    if (oldMode === ChromeVoxMode.CLASSIC && newMode != oldMode) {
       chrome.automation.getFocus((function(focus) {
         if (focus)
           this.setCurrentRange(cursors.Range.fromNode(focus));
       }).bind(this));
     }
+  },
+
+  /**
+ * Toggles between force next and classic/compat modes.
+ * This toggle automatically handles deciding between classic/compat based on
+   * the start of the current range.
+   * @param {boolean=} opt_setValue Directly set to force next (true) or
+   *                                classic/compat (false).
+   * @return {boolean} True to announce current position.
+   */
+  toggleMode: function(opt_setValue) {

[dmazzoni, 2016/06/20 19:49:03]

maybe this should be toggleNext since there are four possible modes but this
just takes two values

[David Tseng, 2016/06/20 22:31:08]

Done.

+    var useClassicOrCompatForCurrentRange = function() {
+      var start;
+      if (this.currentRange && this.currentRange.isValid())
+        start = this.currentRange.start.node;
+      if (!start)
+      return ChromeVoxMode.CLASSIC;

[dmazzoni, 2016/06/20 19:49:03]

indentation

[David Tseng, 2016/06/20 22:31:08]

Done.

+
+      var root = AutomationUtil.getTopLevelRoot(start);
+      if (!root)
+        return ChromeVoxMode.CLASSIC;
+      return this.isWhitelistedForCompat_(root.docUrl) ?
+          ChromeVoxMode.COMPAT : ChromeVoxMode.CLASSIC;
+    }.bind(this);
+
+    var newMode;
+    if (opt_setValue !== undefined) {
+      if (opt_setValue)
+        newMode = ChromeVoxMode.FORCE_NEXT;
+      else
+        newMode = useClassicOrCompatForCurrentRange();
+    } else {
+      if (this.mode_ == ChromeVoxMode.FORCE_NEXT)
+        newMode = useClassicOrCompatForCurrentRange();
+      else
+        newMode = ChromeVoxMode.FORCE_NEXT;
+    }
+    this.setMode(newMode, true);

[dmazzoni, 2016/06/20 19:49:03]

Can you just call refreshMode() here?

It seems like more of this could be refactored to share logic
between refreshMode and toggle.

[David Tseng, 2016/06/20 22:31:08]

RefreshMode differs in the mode transitions it allows. In particular, you cannot
go from force next back to classic/compat. For example, on a load complete, even
though chrome://downloads is whitelisted for compat, force next mode stays
enabled. So, you'd have to call setMode for that case at least.

[dmazzoni, 2016/06/20 22:41:44]

Why not just refreshMode to handle all of those cases, though?

Something like:

constructor:
  refreshMode();

toggleMode:
  var oldMode = this.mode_;
  localStorage['force_next'] = newForceNext;
  refreshMode();
  if (this.mode_ != oldMode) {
    cvox.ChromeVox.tts.speak(announce, ...);
  }

refreshMode:
  if (localStorage['force_next']) {
  } else ...

+
+    var isClassic =
+        newMode == ChromeVoxMode.CLASSIC || newMode == ChromeVoxMode.COMPAT;
+
+    var announce = Msgs.getMsg(isClassic ?
+        'switch_to_classic' : 'switch_to_next');
+
+    cvox.ChromeVox.tts.speak(
+        announce, cvox.QueueMode.FLUSH, {doNotInterrupt: true});
+
+    Notifications.onModeChange();
 
-    this.mode_ = mode;
+    // If the new mode is Classic, return false now so we don't announce
+    // anything more.
+    return newMode != ChromeVoxMode.CLASSIC;
   },
 
   /**
-   * Mode refreshes takes into account both |url| and the current ChromeVox
-   *    range. The latter gets used to decide if the user is or isn't in web
-   *    content. The focused state also needs to be set for this info to be
-   *    reliable.
+   * Refreshes ChromeVox's mode based on a node.
+   * Note that this method disallows switching from force next mode to any other
+   * mode. See toggleMode.
    * @override
    */
   refreshMode: function(node) {
     // Mode changes are based upon the top level web root.
-    var root = node.root;
-    while (root &&
-        root.parent &&
-        root.parent.root &&
-        root.parent.root.role != RoleType.desktop) {
-      root = root.parent.root;
-    }
-
+    var root = AutomationUtil.getTopLevelRoot(node);
     var url = '';
-    if (root && root.role == RoleType.rootWebArea)
+    if (root)
       url = root.docUrl;
 
     var mode = this.mode_;
     if (mode != ChromeVoxMode.FORCE_NEXT) {
       if (this.isWhitelistedForNext_(url)) {
         mode = ChromeVoxMode.NEXT;
-      } else if (this.isBlacklistedForClassic_(url) || (this.currentRange_ &&
-          !this.currentRange_.isWebRange() &&
-          this.currentRange_.start.node.state.focused)) {
+      } else if (this.isWhitelistedForCompat_(url)) {
         mode = ChromeVoxMode.COMPAT;
       } else {
         mode = ChromeVoxMode.CLASSIC;
@@ -339,8 +398,7 @@ Background.prototype = {
       return;
 
     // Is the range invalid?
-    if (newRange.start.node.role === undefined ||
-        newRange.end.node.role === undefined) {
+    if (!newRange.isValid()) {
       // Restore range to the focused location.
       chrome.automation.getFocus(function(f) {
         newRange = cursors.Range.fromNode(f);
@@ -607,26 +665,8 @@ Background.prototype = {
         }
         break;
       case 'toggleChromeVoxVersion':
-        var newMode;
-        if (this.mode_ == ChromeVoxMode.FORCE_NEXT) {
-          var inWeb = current.isWebRange();
-          newMode = inWeb ? ChromeVoxMode.CLASSIC : ChromeVoxMode.COMPAT;
-        } else {
-          newMode = ChromeVoxMode.FORCE_NEXT;
-        }
-        this.setMode(newMode, true);
-
-        var isClassic =
-            newMode == ChromeVoxMode.CLASSIC || newMode == ChromeVoxMode.COMPAT;
-
-        // Leaving unlocalized as 'next' isn't an official name.
-        cvox.ChromeVox.tts.speak(isClassic ?
-            'classic' : 'next', cvox.QueueMode.FLUSH, {doNotInterrupt: true});
-
-        // If the new mode is Classic, return now so we don't announce
-        // anything more.
-        if (newMode == ChromeVoxMode.CLASSIC)
-          return false;
+      if (!this.toggleMode())
+        return false;
         break;
       case 'toggleStickyMode':
         cvox.ChromeVoxBackground.setPref('sticky',
@@ -997,6 +1037,18 @@ Background.prototype = {
   },
 
   /**
+   * Compat mode is on if any of the following are true:
+   * 1. a url is blacklisted for Classic.
+   * 2. the current range is not within web content.
+   * @param {string} url
+   */
+  isWhitelistedForCompat_: function(url) {
+    return this.isBlacklistedForClassic_(url) || (this.currentRange_ &&
+        !this.currentRange_.isWebRange() &&
+        this.currentRange_.start.node.state.focused);
+  },
+
+  /**
    * @param {string} url
    * @return {boolean}
    * @private