WebUI: rework cr-input tabindex and keyboard focus logic

This CL makes it possible to give cr-input a tabindex larger than 0 and
have the tab order work as expected.

Bug: 832177
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I42b25c70ac614360c95140d026755ae66c3cae8c
Reviewed-on: https://chromium-review.googlesource.com/1103002
Commit-Queue: Scott Chen <scottchen@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569450}

chrome/browser/resources/settings/appearance_page/home_url_input.html

@@ -25,7 +25,7 @@
     <cr-input id="input" value="{{value}}" error-message="$i18n{notValid}"
         placeholder="$i18n{enterCustomWebAddress}" maxlength="102400"
         on-change="onChange_" on-keydown="onKeydown_" on-input="validate_"
-        invalid="{{invalid}}" tab-index$="[[getTabindex_(canTab)]]"
+        invalid="{{invalid}}" tabindex="[[getTabindex_(canTab)]]"
         disabled="[[isDisabled_(disabled, pref.*)]]" spellcheck="false"
         on-keyup="stopKeyEventPropagation_"
         on-keypress="stopKeyEventPropagation_">

chrome/browser/resources/settings/people_page/password_prompt_dialog.js

@@ -128,8 +128,8 @@ Polymer({
         // Select the whole password if user entered an incorrect password.
         // Return focus to the password input if it lost focus while being
         // checked (user pressed confirm button).
-        this.$.passwordInput.inputElement.select();
         this.$.passwordInput.focus();
+        this.$.passwordInput.inputElement.select();
         return;
       }
 

chrome/test/data/webui/cr_elements/cr_input_test.js

@@ -7,12 +7,16 @@ suite('cr-input', function() {
   let input;
 
   setup(function() {
+    regenerateNewInput();
+  });
+
+  function regenerateNewInput() {
     PolymerTest.clearBody();
     crInput = document.createElement('cr-input');
     document.body.appendChild(crInput);
     input = crInput.$.input;
     Polymer.dom.flush();
-  });
+  }
 
   test('AttributesCorrectlySupported', function() {
     const attributesToTest = [
@@ -25,17 +29,30 @@ suite('cr-input', function() {
       ['pattern', 'pattern', '', '[a-z]+'],
       ['readonly', 'readOnly', false, true],
       ['required', 'required', false, true],
-      ['tab-index', 'tabIndex', 0, -1],
+      ['tabindex', 'tabIndex', 0, -1],
       ['type', 'type', 'text', 'password'],
     ];
 
     attributesToTest.forEach(attr => {
+      regenerateNewInput();
       assertEquals(attr[2], input[attr[1]]);
       crInput.setAttribute(attr[0], attr[3]);
       assertEquals(attr[3], input[attr[1]]);
     });
   });
 
+  test('togglingDisableModifiesTabIndexCorrectly', function() {
+    crInput.tabindex = 14;
+    assertEquals('14', crInput.getAttribute('tabindex'));
+    assertEquals(14, input.tabIndex);
+    crInput.disabled = true;
+    assertEquals('-1', crInput.getAttribute('tabindex'));
+    assertEquals(-1, input.tabIndex);
+    crInput.disabled = false;
+    assertEquals('14', crInput.getAttribute('tabindex'));
+    assertEquals(14, input.tabIndex);
+  });
+
   test('placeholderCorrectlyBound', function() {
     assertFalse(input.hasAttribute('placeholder'));
 

ui/webui/resources/cr_elements/cr_input/cr_input.html

@@ -95,14 +95,14 @@
         -webkit-appearance: none;
       }
     </style>
-    <div id="label" hidden="[[!label]]">[[label]]</div>
+    <div id="label" hidden="[[!label]]" aria-hidden="true">[[label]]</div>
     <div id="row-container">
       <div id="input-container">
         <!-- Only attributes that are named inconsistently between html and js
              need to use attr$="", such as |tabindex| vs .tabIndex and
              |readonly| vs .readOnly. -->
         <input id="input" disabled="[[disabled]]" autofocus="[[autofocus]]"
-            value="{{value::input}}" tabindex$="[[tabIndex]]" type="[[type]]"
+            value="{{value::input}}" tabindex$="[[tabindex]]" type="[[type]]"
             readonly$="[[readonly]]" maxlength$="[[maxlength]]"
             pattern="[[pattern]]" required="[[required]]"
             incremental="[[incremental]]" minlength$="[[minlength]]">

ui/webui/resources/cr_elements/cr_input/cr_input.js

@@ -15,7 +15,7 @@
  *   placeholder
  *   readonly
  *   required
- *   tabindex as 'tab-index' (e.g.: <cr-input tab-index="-1">)
+ *   tabindex
  *   type (only 'text', 'password', and 'search' supported)
  *   value
  *
@@ -102,7 +102,12 @@ Polymer({
       reflectToAttribute: true,
     },
 
-    tabIndex: String,
+    /** @type {number|undefined} */
+    tabindex: {
+      type: Number,
+      value: 0,
+      reflectToAttribute: true,
+    },
 
     type: {
       type: String,
@@ -125,6 +130,9 @@ Polymer({
     'input.focus': 'onInputFocusChange_',
     'input.blur': 'onInputFocusChange_',
     'input.change': 'onInputChange_',
+    'input.keydown': 'onInputKeydown_',
+    'focus': 'onFocus_',
+    'pointerdown': 'onPointerDown_',
   },
 
   /** @override */
@@ -139,11 +147,21 @@ Polymer({
     return this.$.input;
   },
 
+  /** @private {number} */
+  originalTabIndex_: 0,
+
   /** @private */
   disabledChanged_: function() {
     this.setAttribute('aria-disabled', this.disabled ? 'true' : 'false');
     // In case input was focused when disabled changes.
     this.removeAttribute('focused_');
+
+    if (this.disabled) {
+      this.originalTabIndex_ = /** @type {number} */ (this.tabindex);
+      this.tabindex = -1;
+    } else {
+      this.tabindex = this.originalTabIndex_;
+    }
   },
 
   /**
@@ -159,11 +177,46 @@ Polymer({
       this.inputElement.removeAttribute('placeholder');
   },
 
-  focus: function() {
+  /** @private */
+  onFocus_: function() {
     if (this.shadowRoot.activeElement != this.inputElement)
       this.inputElement.focus();
   },
 
+  /**
+   * Prevents clicking random spaces within cr-input but outside of <input>
+   * from triggering focus.
+   * @param {!Event} e
+   * @private
+   */
+  onPointerDown_: function(e) {
+    // Should not mess with tabindex when <input> is clicked, otherwise <input>
+    // will lose and regain focus, and replay the focus animation.
+    if (e.path[0].tagName !== 'INPUT') {
+      // This is intentionally different from this.originalTabIndex_ since this
+      // only needs to be temporarily stored.
+      const prevTabindex = this.tabindex;
+      this.tabindex = undefined;
+      setTimeout(() => {
+        this.tabindex = prevTabindex;
+      }, 0);
+    }
+  },
+
+  /**
+   * When shift-tab is pressed, first bring the focus to the host element.
+   * This accomplishes 2 things:
+   * 1) Host doesn't get focused when the browser moves the focus backward.
+   * 2) focus now escaped the shadow-dom of this element, so that it'll
+   *    correctly obey non-zero tabindex ordering of the containing document.
+   * TODO(scottchen): check if we still need this after switching to Polymer 2.
+   * @private
+   */
+  onInputKeydown_: function(e) {
+    if (e.shiftKey && e.key === 'Tab')
+      this.focus();
+  },
+
   /** @private */
   onValueChanged_: function() {
     if (this.autoValidate)