Allow navigation to chrome://md-settings/pointer-overlay

chrome/browser/resources/settings/device_page/device_page.js

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview 'settings-device-page' is the settings page for device and
  * peripheral settings.
  */
 Polymer({
   is: 'settings-device-page',
 
   behaviors: [
     I18nBehavior,
     WebUIListenerBehavior,
     settings.RouteObserverBehavior,
   ],
 
   properties: {
     /** Preferences state. */
     prefs: {
       type: Object,
       notify: true,
     },
 
-    /** @private */
-    hasMouse_: {
-      type: Boolean,
-      value: false,
-    },
+    /**
+     * |hasMouse_| and |hasTouchpad_| start undefined so observers don't trigger
+     * until both have been populated.
+     * @private
+     */
+    hasMouse_: Boolean,
 
     /** @private */
-    hasTouchpad_: {
-      type: Boolean,
-      value: false,
-    },
+    hasTouchpad_: Boolean,
 
     /** @private */
     stylusAllowed_: {
       type: Boolean,
       value: function() {
         return loadTimeData.getBoolean('stylusAllowed');
       },
       readOnly: true,
     },
   },
 
   observers: [
     'pointersChanged_(hasMouse_, hasTouchpad_)',
   ],
 
   ready: function() {
-    cr.addWebUIListener('has-mouse-changed', this.set.bind(this, 'hasMouse_'));
-    cr.addWebUIListener(
+    settings.DevicePageBrowserProxyImpl.getInstance().initializePointers();
+  },
+
+  attached: function() {

[dpapad, 2016/12/16 01:20:53]

Nit: @override 

[michaelpg, 2016/12/16 23:32:30]

Done + some other minor closure cleanup.

+    this.addWebUIListener(
+        'has-mouse-changed', this.set.bind(this, 'hasMouse_'));
+    this.addWebUIListener(
         'has-touchpad-changed', this.set.bind(this, 'hasTouchpad_'));
-    settings.DevicePageBrowserProxyImpl.getInstance().initializePointers();
   },
 
   /**
    * @return {string}
    * @private
    */
   getPointersTitle_: function() {
     if (this.hasMouse_ && this.hasTouchpad_)
       return this.i18n('mouseAndTouchpadTitle');
     if (this.hasMouse_)
@@ skipping 50 matching lines @@
   /**
    * Handler for tapping the Storage settings menu item.
    * @private
    */
   onStorageTap_: function() {
     settings.navigateTo(settings.Route.STORAGE);
   },
 
   /** @protected */
   currentRouteChanged: function() {
     this.checkPointerSubpage_();

[dpapad, 2016/12/16 01:20:53]

Is this call necessary? IIUC sometimes the desired behavior when visiting
chrome://md-settings/pointer-overlay is to immediately navigate elsewhere? From
a UX perspective, I would expect to stay in whatever page I explicitly navigated
(even if everything on that page is disabled).

[michaelpg, 2016/12/16 23:32:31]

Oops, I replied to this as a message.

[dpapad, 2016/12/17 01:05:08]

Preferably there would be a message "Nothing to see here", or "No devices", so
we wouldn't actually display nothing, but also would not surprise the user by
magically navigating to the parent page.

[michaelpg, 2016/12/17 03:04:47]

Sure, that's doable - "patches welcome" :-)

This CL is fixing objectively broken behavior (loading
chrome://md-settings/pointer-overlay directly always redirects to the device
section, 100% of the time).

Removing the existing behavior of closing the subpage when there are no pointer
devices, by keeping the page open with a "You can't do anything here, go away"
message, should be a separate CL. We'd also need a new string or two at least.

   },
 
   /**
    * @param {boolean} hasMouse
    * @param {boolean} hasTouchpad
    * @private
    */
   pointersChanged_: function(hasMouse, hasTouchpad) {
     this.$.pointersRow.hidden = !hasMouse && !hasTouchpad;
     this.checkPointerSubpage_();
   },
 
   /**
    * Leaves the pointer subpage if all pointing devices are detached.
    * @private
    */
   checkPointerSubpage_: function() {
-    if (!this.hasMouse_ && !this.hasTouchpad_ &&
+    // Check that the properties have explicitly been set to false.
+    if (this.hasMouse_ === false && this.hasTouchpad_ === false &&
         settings.getCurrentRoute() == settings.Route.POINTERS) {
       settings.navigateTo(settings.Route.DEVICE);
     }
   },
 });