cros: Allow pin keyboard to keep focus without popping up the pin keyboard.

chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js

Index: chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
diff --git a/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js b/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
index d8b2762fb104855a57eb10cc72b4413c231f46ea..fb1c919f8b8011928e2612bdcc5b6aa78e0fdebd 100644
--- a/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
+++ b/chrome/browser/resources/chromeos/quick_unlock/pin_keyboard.js
@@ -12,14 +12,13 @@
  *           the PIN keyboard's value.
  *
  * Events:
- *    pin-change: Fired when the PIN value has changed. The pin is available at
+ *    pin-change: Fired when the PIN value has changed. The PIN is available at
  *                event.detail.pin.
- *    submit: Fired when the PIN is submitted. The pin is available at
+ *    submit: Fired when the PIN is submitted. The PIN is available at
  *            event.detail.pin.
  *
  * Example:
- *    <pin-keyboard on-pin-change="onPinChange" on-submit="onPinSubmit"
- *                  value="{{pinValue}}">
+ *    <pin-keyboard on-pin-change="onPinChange" on-submit="onPinSubmit">
  *    </pin-keyboard>
  */
 
@@ -40,6 +39,14 @@ var REPEAT_BACKSPACE_DELAY_MS = 150;
  */
 var INITIAL_BACKSPACE_DELAY_MS = 500;
 
+/**
+ * The key codes of the keys allowed to be used on the pin input, in addition to
+ * number keys. Currently we allow backspace(8), tab(9), left(37) and right(39).
+ * @type {Array<number>}
+ * @const
+ */
+var PIN_INPUT_ALLOWED_NON_NUMBER_KEY_CODES = [8, 9, 37, 39];
+
 Polymer({
   is: 'pin-keyboard',
 
@@ -51,6 +58,7 @@ Polymer({
     /**
      * Whether or not the keyboard's input element should be numerical
      * or password.
+     * @private
      */
     enablePassword: {
       type: Boolean,
@@ -58,14 +66,21 @@ Polymer({
     },
 
     /**
-     * Whether or not the keyboard's input should be shown.
+     * The password element the pin keyboard is associated with. If this is not
+     * set, then a default input element is shown and used.
+     * @type {HTMLInputElement}
+     * @private
      */
-    hideInput: {
-      type: Boolean,
-      value: false
+    passwordElement: {
+      type: Object,
+      value: function() { return this.$$('#pin-input'); },
+      observer: 'onPasswordElementAttached_'
     },
 
-    /** The value stored in the keyboard's input element. */
+    /**
+     * The value stored in the keyboard's input element.
+     * @private
+     */
     value: {
       type: String,
       notify: true,
@@ -93,37 +108,90 @@ Polymer({
   },
 
   /**
-   * @override
+   * Called when a password element is attached to the pin keyboard.
+   * @param {HTMLInputElement} inputElement The PIN keyboard's input element.
+   * @private
+   */
+  onPasswordElementAttached_: function(inputElement) {
+    if (inputElement != this.$$('#pin-input'))
+      this.$$('#pin-input').hidden = true;
+    inputElement.addEventListener('input',
+        this.handleInputChanged_.bind(this));
+  },
+
+  /**
+   * Called when the user uses the keyboard to enter a value into the input
+   * element.
+   * @param {Event} event The event object.
+   * @private
+   */
+  handleInputChanged_: function(event) {
+    this.value = event.target.value;
+  },
+
+  /**
+   * Gets the selection start of the input field.
+   * @type {number}
+   * @private
+   */
+  get selectionStart_() {
+    return this.passwordElement.selectionStart;
+  },
+
+  /**
+   * Gets the selection end of the input field.
+   * @type {number}
+   * @private
+   */
+  get selectionEnd_() {
+    return this.passwordElement.selectionEnd;
+  },
+
+  /**
+   * Sets the selection start of the input field.
+   * @param {number} start The new selection start of the input element.
+   * @private
    */
-  attached: function() {
-    // Remove the space/enter key binds from the polymer
-    // iron-a11y-keys-behavior.
-    var digitButtons = Polymer.dom(this.root).querySelectorAll('.digit-button');
-    for (var i = 0; i < digitButtons.length; ++i)
-      digitButtons[i].keyEventTarget = null;
+  set selectionStart_(start) {
+    this.passwordElement.selectionStart = start;
   },
 
   /**
-   * Gets the container holding the password field.
-   * @type {!HTMLInputElement}
+   * Sets the selection end of the input field.
+   * @param {number} end The new selection end of the input element.
+   * @private
    */
-  get inputElement() {
-    return this.$$('#pin-input');
+  set selectionEnd_(end) {
+    this.passwordElement.selectionEnd = end;
   },
 
   /** Transfers focus to the input element. */
   focus: function() {
-    this.$$('#pin-input').focus();
+    this.passwordElement.focus();
   },
 
   /**
    * Called when a keypad number has been tapped.
-   * @param {!{target: !PaperButtonElement}} event
+   * @param {Event} event The event object.
    * @private
    */
-  onNumberTap_: function(event, detail) {
+  onNumberTap_: function(event) {
     var numberValue = event.target.getAttribute('value');
-    this.value += numberValue;
+
+    // Add the number where the caret is, then update the selection range of the
+    // input element.
+    this.value = this.value.substring(0, this.selectionStart_) + numberValue +
+        this.value.substring(this.selectionEnd_);
+    this.selectionStart_++;
+    this.selectionEnd_ = this.selectionStart_;
+
+    // If a number button is clicked, we do not want to switch focus to the
+    // button, therefore we transfer focus back to the input, but if a number
+    // button is tabbed into, it should keep focus, so users can use tab and
+    // spacebar/return to enter their PIN.
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /** Fires a submit event with the current PIN value. */
@@ -138,8 +206,17 @@ Polymer({
    * @param {string} previous
    */
   onPinValueChange_: function(value, previous) {
-    if (value != previous)
+    if (value != previous) {
+      // The selection caret gets placed at the end after altering the
+      // password element, so we store the previous location(s) and reapply
+      // them after the the new value is set.

[xiyuan, 2016/11/09 18:06:32]

Why do we need to reapply the selection range? What if the new value is totally
unrelated to the old value. Wouldn't the default behavior make more sense in
such case?

I guess the logic is intended to go with the 'input' event, maybe move the logic
to handleInputChanged_ ?

[sammiequon, 2016/11/10 21:06:01]

The default behavior works fine if we are just working with the input element
and keyboard. It is the number and clear buttons which we have to manually alter
the input elements value that changes selection range to be at the end.

[xiyuan, 2016/11/10 21:48:35]

Thanks for the clarification. I am still not convinced the selection range
caching logic belongs here.

Suppose the current value is "123" with caret at the end.
What happens when we set the value to "abcd"? Would the logic give us "abc|d"
(where "|" is the caret)?

If the problem is with onNumberTap_ and onPinClear_, we should move the logic
there.

[sammiequon, 2016/11/10 22:36:56]

I didn't test setting the value(forgot about that case) but I think it would
probably be abc|d. Anyhow, I moved the caching logic into onNumberTap_ and
onPinClear_.

+      var selectionStart = this.selectionStart_;
+      var selectionEnd = this.selectionEnd_;
+      this.passwordElement.value = this.value;
+      this.selectionStart_ = selectionStart;
+      this.selectionEnd_ = selectionEnd;
       this.fire('pin-change', { pin: value });
+    }
   },
 
   /**
@@ -148,7 +225,17 @@ Polymer({
    * @private
    */
   onPinClear_: function() {
-    this.value = this.value.substring(0, this.value.length - 1);
+    // If the input is shown, clear the text based on the caret location or
+    // selected region of the input element. If it is just a caret, remove the
+    // character in front of the caret.
+    if (this.selectionStart_ == this.selectionEnd_)
+      this.selectionStart_--;
+
+    this.value = this.value.substring(0, this.selectionStart_) +
+        this.value.substring(this.selectionEnd_);
+
+    // Move the caret or selected region to the correct new place.
+    this.selectionEnd_ = this.selectionStart_;
   },
 
   /**
@@ -163,6 +250,10 @@ Polymer({
         this.repeatBackspaceIntervalId_ = setInterval(
             this.onPinClear_.bind(this), REPEAT_BACKSPACE_DELAY_MS);
     }.bind(this), INITIAL_BACKSPACE_DELAY_MS);
+
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /**
@@ -184,6 +275,10 @@ Polymer({
    */
   onBackspacePointerOut_: function(event) {
     this.clearAndReset_();
+
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /**
@@ -198,82 +293,69 @@ Polymer({
     if (!this.repeatBackspaceIntervalId_)
       this.onPinClear_();
     this.clearAndReset_();
-  },
-
-  /** Called when a key event is pressed while the input element has focus. */
-  onInputKeyDown_: function(event) {
-    // Up/down pressed, swallow the event to prevent the input value from
-    // being incremented or decremented.
-    if (event.keyCode == 38 || event.keyCode == 40) {
-      event.preventDefault();
-      return;
-    }
 
-    // Enter pressed.
-    if (event.keyCode == 13) {
-      this.firePinSubmitEvent_();
-      event.preventDefault();
-      return;
-    }
+    if (!event.target.receivedFocusFromKeyboard)
+      this.focus();
+    event.preventDefault();
   },
 
   /**
-   * Keypress does not handle backspace but handles the char codes nicely, so we
-   * have a seperate event to process the backspaces.
-   * @param {Event} event Keydown Event object.
+   * Helper function to check whether a given |event| should be processed by
+   * the numeric only input.
+   * @param {Event} event The event object.
    * @private
    */
-  onKeyDown_: function(event) {
-    // Backspace pressed.
-    if (event.keyCode == 8) {
-      this.onPinClear_();
-      event.preventDefault();
-      return;
-    }
+  isValidEventForInput_: function(event) {
+    // Valid if the key is a number, and shift is not pressed.
+    if ((event.keyCode >= 48 && event.keyCode <= 57) && !event.shiftKey)
+      return true;
+
+    // Valid if the key is one of the selected special keys defined in
+    // |PIN_INPUT_ALLOWED_NON_NUMBER_KEY_CODES|.
+    if (PIN_INPUT_ALLOWED_NON_NUMBER_KEY_CODES.indexOf(event.keyCode) > -1)
+      return true;
+
+    // Valid if a is pressed while the control key is pressed to allow users to

[xiyuan, 2016/11/09 18:06:32]

nit: "a is pressed while the control key is pressed"
-> "the key is CTRL+A"

[sammiequon, 2016/11/10 21:06:01]

Done.

+    // quickly select the entire PIN.
+    if (event.keyCode == 65 && event.ctrlKey)
+      return true;
+
+    // The rest of the keys are invalid.
+    return false;
   },
 
   /**
-   * Called when a key press event is fired while the number button is focused.
-   * Ideally we would want to pass focus back to the input element, but the we
-   * cannot or the virtual keyboard will keep poping up.
-   * @param {Event} event Keypress Event object.
+   * Called when a key event is pressed while the input element has focus.
+   * @param {Event} event The event object.
    * @private
    */
-  onKeyPress_: function(event) {
-    // If the active element is the input element, the input element itself will
-    // handle the keypresses, so we do not handle them here.
-    if (this.shadowRoot.activeElement == this.inputElement)
+  onInputKeyDown_: function(event) {
+    // Up/down pressed, swallow the event to prevent the input value from
+    // being incremented or decremented.
+    if (event.keyCode == 38 || event.keyCode == 40) {
+      event.preventDefault();
       return;
-
-    var code = event.keyCode;
+    }
 
     // Enter pressed.
-    if (code == 13) {
+    if (event.keyCode == 13) {
       this.firePinSubmitEvent_();
       event.preventDefault();
       return;
     }
 
-    // Space pressed. We want the old polymer function of space activating the
-    // button with focus.
-    if (code == 32) {
-      // Check if target was a number button.
-      if (event.target.hasAttribute('value')) {
-        this.value += event.target.getAttribute('value');
-        return;
-      }
-      // Check if target was backspace button.
-      if (event.target.classList.contains('backspace-button')) {
-        this.onPinClear_();
-        return;
-      }
+    // Do not pass events that are not numbers or special keys we care about. We
+    // use this instead of input type number because there are several issues
+    // with input type number, such as no selectionStart/selectionEnd and
+    // entered non numbers causes the caret to jump to the left.
+    if (!this.isValidEventForInput_(event)) {
+      event.preventDefault();
+      return;
     }
-
-    this.value += String.fromCharCode(code);
   },
 
   /**
-   * Disables the submit and backspace button if nothing is entered.
+   * Disables the backspace button if nothing is entered.
    * @param {string} value
    * @private
    */
@@ -282,16 +364,6 @@ Polymer({
   },
 
   /**
-   * Computes whether the input type for the pin input should be password or
-   * numerical.
-   * @param {boolean} enablePassword
-   * @private
-   */
-  getInputType_: function(enablePassword) {
-    return enablePassword ? 'password' : 'number';
-  },
-
-  /**
    * Computes the value of the pin input placeholder.
    * @param {boolean} enablePassword
    * @private