Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

paper buttons get unexpected focus on Android and iOS Chrome #80

Open
2 of 3 tasks
jab opened this issue Jun 13, 2017 · 10 comments
Open
2 of 3 tasks

paper buttons get unexpected focus on Android and iOS Chrome #80

jab opened this issue Jun 13, 2017 · 10 comments

Comments

@jab
Copy link

jab commented Jun 13, 2017

Expected outcome

The button is not focused.

Actual outcome

The button is focused.

Live Demo

https://jab.github.io/paper-behaviors-issue-80/

source: https://github.com/jab/paper-behaviors-issue-80/

Steps to reproduce

  1. Open the live demo in Chrome for Android or iOS (does not reproduce in iOS Safari)
  2. Click the "go to page 2" link as directed
  3. Click the back button as directed

Browsers Affected

  • Chrome v58.0.3029.83 on Android 6.0.1 (Nexus 5 Build/M4B30Z)
  • Chrome v59.0.3071.84 on iOS 10.3.2 (iPhone 7 plus)
  • Safari on iOS 10.3.2 (iPhone 7 plus)

Hypothesis

Looking at https://github.com/PolymerElements/paper-behaviors/blob/fd8c137/paper-inky-focus-behavior.html#L26-L33, I think what's happening is that for some reason, the receivedFocusFromKeyboard param is getting passed in as true, even though the only user interactions have been taps (there's no keyboard in sight).

Screenshots

Chrome for iOS

iOS

Chrome for Android

Android

@jab jab changed the title paper button gets unexpected focus on Android and iOS Chrome paper buttons get unexpected focus on Android and iOS Chrome Jun 13, 2017
@jab
Copy link
Author

jab commented Jun 13, 2017

(Changed the live demo link in the description from the plnkr.co link I was originally using, since it was flaky)

@notwaldorf
Copy link
Contributor

Hmmm, this sounds a lot like PolymerElements/paper-button#24. Do you think it's the same?

@jab
Copy link
Author

jab commented Jun 14, 2017

It looks like it could be related, though I've never triggered this by switching browser tabs.

FWIW, you can reproduce this in desktop Chrome by turning on mobile device emulation:

screen shot 2017-06-13 at 22 28 27

Looking further, I just set a breakpoint in PaperInkyFocusBehaviorImpl's _focusChanged(..) to see why receivedFocusFromKeyboard is getting set to true. Walking up the call stack, I came to IronButtonStateImpl's _detectKeyboardFocus(..), where I noticed that the focused param it's getting passed is true:

screen shot 2017-06-13 at 22 43 55

Walking up the stack a little further, I came to IronControlState's _focusBlurHandler(..), which is what's incorrectly setting focused on this button to true in the first place:

screen shot 2017-06-13 at 22 38 05

At that point my call stack ends, so I'm not immediately sure what's triggering _focusBlurHandler(..) on this button. If I can figure that out, maybe I can figure out how to stop this from happening in the first place. If you have any ideas, please let me know. Thanks!

@jab
Copy link
Author

jab commented Jun 14, 2017

@notwaldorf I noticed that with the following patch against my repro, the bug goes away:

diff --git a/app-root.html b/app-root.html
index 7918ac2..b6a5de6 100644
--- a/app-root.html
+++ b/app-root.html
@@ -101,7 +101,7 @@
       changePage: function (page) {
         this.async(function () {
           this.set('pageData.page', page);
-        });
+        }, 250);
       },
       _computePage: function (pageData) {
         return pageData.page;

Is this somehow a helpful clue in fixing the bug?

@e111077
Copy link
Contributor

e111077 commented Jun 27, 2017

Looking into this deeper, it seems to be an issue with our state machine in iron-button-state. Due to the sticky behavior of a tap, the button under the finger while you tap changes due to the javascript, and thus a new button is focused.

The problem is that since this is a new button with a different instance of iron-button-state, it cannot tell that the user has their finger down and thus believes that the pointer is up and the button is focused. Thus, the element thinks that this was a keyboard focus.

This is why when you change the state with the async, it gives the user enough time to lift their finger before it changes to a new button. Thus, the element does not gain focus on attached.

TL;DR: The user enters the state machine of iron-button-state at the wrong point

@jab
Copy link
Author

jab commented Jun 28, 2017

Thanks for taking a look at this @e111077!

That explanation makes it sound to me like the finger is being held down in the same spot while button A is swapped out for button B, which is what's causing button B to think there's a finger over it, and therefore that it should be focused.

But when I hold my finger down over button A, nothing happens -- that is, it's not actually swapped out for button B -- until I lift my finger off of it. Which is how it seems like on-tap is supposed to work.

Given that, I'm not following why the system ever thinks there's a finger over button B. Am I misunderstanding?

In any case, it looks like we agree that the line this._setFocused(event.type === 'focus') (highlighted in my most recent screenshot) is problematic. That ends up amounting to _setFocused(true), even though the button should not be focused. Not immediately sure how to change the logic though to fix this. Do you have any ideas?

@jab
Copy link
Author

jab commented Jun 28, 2017

Meant to say, one other thing that would be interesting to address here is why the bug manifests in some environments (e.g. Chrome for iOS and Android) but not others (mobile Safari).

@jab
Copy link
Author

jab commented Sep 26, 2017

We upgraded to Polymer 2 and are still seeing this behavior.

I realized I could create an even simpler reproduction using alert:
https://cdn.rawgit.com/jab/paper-behaviors-issue-80/f46a9164/polymer2.html

The source for this is at https://github.com/jab/paper-behaviors-issue-80/blob/master/polymer2.html

@notwaldorf @e111077 et al., do you have any ideas for a fix or workaround?

Thanks so much for taking a look!

@jab
Copy link
Author

jab commented Sep 26, 2017

Meant to say, this new test case reproduces the bug for me no matter what browser or platform I test with. I assume it's the same bug with incorrect "using the keyboard?" detection, but please let me know if it's a separate issue.

@keanulee
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants