When panning, the link under the tap is briefly highlighted before the panning starts. The tap should not be interpreted at all.

We intentionally provide highlighting as feedback as soon as you press down, to improve perceived responsiveness and to let you know what will happen if you complete the tap (so you have a chance to move your finger if it wasn't what you wanted to click).

I think we have to do this to feel responsive to users. Marking this invalid for now.

Why would you suggest that you are going to open a link if you are not? To me that has the opposite effect: I want to pan but I see a link highlighted. Panic. No no no stop opening, cancel. i just want to pan!

(In reply to Stefan Arentz [:st3fan] from comment #3) > Why would you suggest that you are going to open a link if you are not? At the time the user touches the screen, we don't know whether we are going to follow the link. We won't know that until at least a half-second later, because we have to wait to see whether the user single-taps, long-taps, double-taps, or pans. If we wait that long before giving any feedback, the lack of feedback adds its own confusion. Maybe we could make this a little less sensitive, by adding a small delay before the tap highlight (maybe 10ms?) -- enough so that it won't happen on a quick swipe gesture, but not enough to hurt perceived responsiveness. Other mobile browsers also have tap highlights that vanish if the tap becomes a pan. The stock browser on ICS works similar to Firefox, with a highlight that appears almost immediately. Chrome for Android has a noticeable delay before the highlight appears, and so do Opera Mobile and the stock Gingerbread browser.

Yeah that makes sense Matt. I think it is indeed a sensitivity issue. I do see the same behaviour on iOS/Safari but I think the grace period there is maybe a little longer. Not super long though, it is likely in the 10 to 50ms range or so.

Adjusting the title to something more specific then

Based on trial and error, 50ms indeed seems to be a good compromise. It doesn't completely eliminate premature highlights, but it makes them rather less common without any significant effect on responsiveness.

Comment on attachment 641301 [details] [diff] [review] patch Review of attachment 641301 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Let's try it. ::: mobile/android/chrome/content/browser.js @@ +3466,5 @@ > _scrollableElement: null, > > _highlightElement: null, > > + _timeout: null, Lets use _highlightTimer or _highlightTimeout @@ +3473,5 @@ > + this._cancelTapHighlight(); > + this._timeout = setTimeout(function(self) { > + DOMUtils.setContentState(aElement, kStateActive); > + self._highlightElement = aElement; > + }, 50, this); Move this 50ms constant somewhere

Pushed with review comments fixed: https://hg.mozilla.org/integration/mozilla-inbound/rev/7d11ab4b1a8c

Backed out because of bug 778333 and possibly other regressions: https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef483246388 In bug 775372 I requested Aurora approval for the backout.

Backed out on Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/a793894365e7

(In reply to Matt Brubeck (:mbrubeck) from comment #12) > Backed out because of bug 778333 and possibly other regressions: > https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef483246388 > > In bug 775372 I requested Aurora approval for the backout. https://hg.mozilla.org/mozilla-central/rev/4ef483246388

Is it possible to put this back in? I really liked this change. It made a big difference to me.

I haven't had time to investigate the regressions properly. If someone else has time before I do (Wes?), feel free to steal this bug from me.

I wonder if we can set the tapHighlight item but delay setting active on it. I think that might fix these races.

This is bug is also showing up when you scroll a selectable list of things. For example the list of tabs in Firefox itself or on the Mozilla Marketplace, when you scroll the list of apps. The effect is the same, the list item is selected and then deselected a little while later when you are scrolling the list. In other words, it does not just happen with links, but with any clickable element?

So I THINK that most of the problems we had before were because we weren't setting _highlightElement immediately when you tapped. That led to races where certain events expect a highlightElement and if it doesn't exist they.... fail. So this is basically the same patch except it sets highlightElement regardless, and just delays setting :active on it for some timeout. Also I kept getting into situations where we'd set the highlight on something that had no ownerDocument (not sure what....) and then throw leading us to never reset the highlightElement and breaking.... everything. So I added a check for the elt.ownerDocument.

Comment on attachment 655676 [details] [diff] [review] Patch >+ _timeout: null, In comment 9 you asked me to use "_highlightTimeout" >+ this._timeout = setTimeout(function() { >+ DOMUtils.setContentState(aElement, kStateActive); >+ }, 50); ...and to make this a named constant. I also think you might need to check the ownerDocument before calling setContentState, like we do in _cancelTapHighlight. See the original patch from bug 775372, for example. (In reply to Wesley Johnston (:wesj) from comment #19) > Also I kept getting into situations where we'd set the highlight on > something that had no ownerDocument (not sure what....) and then throw > leading us to never reset the highlightElement and breaking.... everything. > So I added a check for the elt.ownerDocument. This might be similar to what happened in bug 775372, when the timeout fired after the original page has been unloaded. This code looks good, but please test thoroughly to make sure bug 775372 and bug 778333 (or similar situations) don't recur, and that no uncaught exceptions are logged (in particular if a new page loads while the highlight is showing or about to show).

https://hg.mozilla.org/integration/mozilla-inbound/rev/58b937c070d6 For those wondering, mbrubeck did not r+ his own patch even though the commit says he did. I just forgot to hg qref -U it.

Channel uplift?

I saw a report today of this causing problems. Don't want to uplift yet.

backed out ^

Not actively working on this. The races here are difficult to figure out, but if someone wants to give it a shot, feel free.

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.