Popup: "highlight" flash during popup show on Android 4.1, 4.2 #7533

Closed
sterlingnelson opened this Issue Jun 30, 2014 · 14 comments

Projects

None yet

4 participants

@sterlingnelson

Description/current behavior
Before a popup finishes showing in Android 4.1 or 4.2, there is a brief orange or blue "highlight" style flash in the area that the popup is to occupy.

"Expected" (Hoped for) behavior
No transient flash occurs while opening the popup.

Affected Platforms
We're building a Cordova app, and have produced this behavior on the Android stock browser on:

  1. a Samsung Galaxy Tab S3 running Android 4.1.2 produces an orange flash in both WebView and the "Internet" browser app.
  2. a Kindle Fire HDX 7" running FireOS 3 (Android 4.2.2 fork) produces a blue flash in WebView only (browser app is Silk)

We have not observed this behavior in other browsers. We are using jQuery Core 1.11.0 and jQuery Mobile 1.4.0 for our project.

JSBin
Our Samsung device reproduces the described behavior in its stock browser using the current JSBin template.
http://jsbin.com/huvoraba/27

@ldeluca
Contributor
ldeluca commented Sep 29, 2014

I tried reproducing with an Apache Cordova app. Cordova version 3.5 on my android Samsung Galaxy S4. I do not see an orange or blue "highlight" flash.

@SterlingAtXM

Thanks for taking the time to test and respond!

We don't have an Android 4.4 device here, but I think I tested on a Nexus running 4.4 and did not see the flashes there either. I have been assuming it's a difference between the KitKat(Chromium based) and pre KitKat WebViews. I've only seen it in 4.1 and 4.2, although I haven't had the chance to test earlier versions.

Edit: Sorry, should have said, we're running Cordova 3.5.0 here as well.

@ldeluca
Contributor
ldeluca commented Oct 6, 2014

I was able to test in browserStack and I see the issue you were referring to now. I uploaded a quick video to youtube showing the blue flash. I tested on a 4.2 webview for android. https://www.youtube.com/watch?v=KnmTzKLw8ME&feature=youtube_gdata_player

@ldeluca
Contributor
ldeluca commented Oct 6, 2014

So my sample page has two types of popups. The first is a button based popup and the second is a link based. The button is the one that shows the blue flash. The link version does not. I noticed that there was a difference in the use of class="popup" so I updated my test page to test to see if that would make the button flash. It still flashed without that class on a button popup. I also added a link button to see if there would be a blue flash on a link with the class set. It does NOT flash. So my conclusion is: The flash ONLY occurs on button based popups, NOT link based. During the link, the link itself glows blue as if it were selected before the popup. With button based the popup glows blue.

@gabrielschulhof
Contributor

@ldeluca OK, excellent observation. If this is the case, then popups must indeed be opened via the pagecontainer "change" method, because that introduces sufficient "distance" (in terms of asynchronicity) between the click event and the opening of the popup. I implemented that for the custom selectmenu in b0a802d.

@gabrielschulhof
Contributor

... or so I've thought ... I've tried simulating what we do during the link handling with custom code in http://jsbin.com/huvoraba/158/, and I find that both the custom code and jQuery Mobile's link handling cause the blue flash: https://www.youtube.com/watch?v=zlYq65zjuj0

@gabrielschulhof
Contributor

@ldeluca if you wanna do a PR to fix this issue, you wanna look at the effect of removing

https://github.com/jquery/jquery-mobile/blob/master/js/widgets/popup.js#L604

Try removing it or unwinding the stack before focusing.

The reason for the .focus() call there is to put the focus on an element (the popup's container div) that will be a valid element in the focus-restricted environment that the popup produces. It produces this environment by handling the focusin event at the document level while it's open

https://github.com/jquery/jquery-mobile/blob/master/js/widgets/popup.js#L289

and transferring focus to an element inside the popup if it finds that an element that is not the child of the popup container has received focus.

To make sure your solution works correctly, you must test it on a page that includes a text input which can receive the focus by repeatedly pressing the Tab key on the page and a popup. Whenever the popup is on the screen, the text input must not be reachable via either the mouse or the keyboard.

Note that you can still trick Firefox (and maybe Chrome) into giving the text input focus by typing some text into it, then opening the popup, then searching on the page for the text you've typed. This works especially well in Firefox because of its type-to-search feature. There's no way around this (that I know of) so don't worry if that breaks the focus restriction.

@gabrielschulhof
Contributor

The video shows a browserstack instance of a Samsung Galaxy Note II running Android 4.1.

@ldeluca
Contributor
ldeluca commented Oct 6, 2014

@gabrielschulhof I tried removing the call to _desiredCoords and tested but am still seeing the blue flash. Removing the _desiredCoords call just put the popup near the button instead of at the bottom of the page. Seems like a different issue?

It looks like the issue is more about the style that is being applied. The blue glow must be a :focus css style somewhere is my guess. You sure line https://github.com/jquery/jquery-mobile/blob/master/js/widgets/popup.js#L604 is the right spot to be looking?

@gabrielschulhof
Contributor

@ldeluca I'm sooo sorry! I mistyped! I meant this:

https://github.com/jquery/jquery-mobile/blob/master/js/widgets/popup.js#L642

That is, the line that calls .focus() on the popup container element after the popup has finished opening.

I'm thinking though that the line cannot be removed because whatever element is focused before the popup opens cannot remain focused, otherwise we cannot claim that the popup restricts focus while open.

Trying different values for the popup containers focus style might not be such a bad idea though!

@ldeluca
Contributor
ldeluca commented Oct 7, 2014

Looking into this from a styling perspective. Looks like there are similar concerns out on the innerwebs :) https://forum.jquery.com/topic/blue-highlight-over-popup

only happens to Button popups

@ldeluca
Contributor
ldeluca commented Oct 7, 2014

@gabrielschulhof when I add a timeout before the call to .focus then i don't see the blue flash anymore:

setTimeout(function() {
this._ui.container.attr( "tabindex", "0" ).focus();
}, 300);

Is my solution and it appears to work but running grunt test shows some failures.

@ldeluca
Contributor
ldeluca commented Oct 8, 2014

I used an alternative solution per @gabrielschulhof suggestion to blur out the other elements that are not within the popup. I updated the test case to verify that an input field that has focus before the popup becomes blurred after the popup gains focus. The pull request can be found here: #7757

@SterlingAtXM

Just a quick thanks to @ldeluca and @gabrielschulhof. I appreciate the time you guys are putting in!

@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 9, 2014
@ldeluca ldeluca Popup: test case add an assertion for input focus\n\nFixes #7533 893f076
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 9, 2014
@ldeluca ldeluca Popup: add safelyBlur() method\n\nFixes #7533 f8f465c
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 9, 2014
@ldeluca ldeluca Popup: add safelyBlur() method\n\nFixes #7533 e44ba48
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 9, 2014
@ldeluca ldeluca Popup: blue focus flash on android 4.2
replaced the contains check with a jquery friendly version

Ref #7533
12e3db8
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: fixed logic for scoping and added assertions
Fixes gh-7533
6e9ad3b
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates
Fixes gh-7533
5ba1121
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates
Fixes gh-7533
827f9ee
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates
Fixes gh-7533
287d406
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates whitespace issues
Fixes gh-7533
2127bd1
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates whitespace issues
Fixes gh-7533
01acec8
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates whitespace issues
Fixes gh-7533
02da3b4
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates comment capitalization
Fixes gh-7533
b08fc5b
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates comment capitalization
Fixes gh-7533
a94e529
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 14, 2014
@ldeluca ldeluca Popup: styling updates comment space after inline comment begins
Fixes gh-7533
cc548fd
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 15, 2014
@ldeluca ldeluca Popup: replaced activeElement with targetElement for correct focus
Fixes gh-7533
3e5ad2d
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 15, 2014
@ldeluca ldeluca Popup: contains check was wrong, fixed.
Fixes gh-7533
804a8ea
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 15, 2014
@ldeluca ldeluca Popup: currentElement update for blur, fixed.
Fixes gh-7533
66e5702
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 16, 2014
@ldeluca ldeluca Popup: updated the safelyBlur method Gabriels feedback. TESTS FAIL.
Fixes gh-7533
3118be2
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 16, 2014
@ldeluca ldeluca Popup: updated openPrerequisitesComplete method. TESTS PASS.
Fixes gh-7533
4fe66a0
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 16, 2014
@ldeluca ldeluca Popup: updated openPrerequisitesComplete method. TESTS PASS.
Fixes gh-7533
cc6e6cd
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 17, 2014
@ldeluca ldeluca Popup: fixed too long code lines.
Fixes gh-7533
51f427a
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 20, 2014
@ldeluca ldeluca Popup: fixed && causing build break.
Fixes gh-7533
6a5ff2c
@ldeluca ldeluca added a commit to ldeluca/jquery-mobile that referenced this issue Oct 24, 2014
@ldeluca ldeluca Popup: fixed minor spacing issues. \n\nFixes #7533 d2eedd9
@gabrielschulhof gabrielschulhof added a commit that referenced this issue Oct 24, 2014
@ldeluca @gabrielschulhof ldeluca + gabrielschulhof Popup: Blue focus flash on android 4.2
(cherry picked from commit 59fac36)

Closes gh-7757
Fixes gh-7533
545b66e
@agcolom agcolom added a commit to agcolom/jquery-mobile that referenced this issue Nov 26, 2014
@ldeluca @agcolom ldeluca + agcolom Popup: Blue focus flash on android 4.2
Closes gh-7757
Fixes gh-7533
8ba1b03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment