Closed Bug 941324 Opened 11 years ago Closed 11 years ago

Double-tap on a link should not open that link

Categories

(Firefox for Metro Graveyard :: Pan and Zoom, defect)

All
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 28

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(2 files)

Since we are using double-tap to zoom, we should make sure that double-taps does not pass a "click" event onto content where it might accidentally activate a link or button when you are trying to zoom to it.
Summary: Double-tap on a link should not open that link → Defect - Double-tap on a link should not open that link
Whiteboard: [triage] feature=defect c=tbd u=tbd p=0
No longer blocks: metrov1backlog
Summary: Defect - Double-tap on a link should not open that link → Double-tap on a link should not open that link
Whiteboard: [triage] feature=defect c=tbd u=tbd p=0
To fix this correctly, I think we need to stop dispatching mouse events to content on double-tap (see bug 936754).  Instead we should wait for gesture recognition before dispatching any events.  If a double-tap is recognized then we should *not* dispatch mouse events.
Assignee: nobody → mbrubeck
Hardware: x86_64 → All
We can take that SystemParametersInfo call for Windows compact for a spin and see if it works on Win8. If not, we might be able to grab these values from the registry at

HKCU/Software/Microsoft/Wisp/Touch

For double tap there are a couple values - 

HKCU/Software/Microsoft/Wisp/Touch
TouchModeN_DtapTime = (dword)
TouchModeN_DtapDist = (dword)

I was hoping TouchModeN_DtapTime was the delay before a single tap is sent, but for some reason this value seemed reversed from what yoi would expect. It decreases if you increase the double tap time. For the slowest setting it's 0, for the fastest it's 100.

If we have too we can file a support request on this.
Depends on: 933990
Currently we send the touchstart and first touchmove to APZC, but if HitTestChrome returns true then we always skip the APZC after that.  Since we never actually use the APZC for anything in touches that hit chrome, this just skips APZC entirely for that case.
Attachment #8336984 - Flags: review?(jmathies)
This uses APZC's gesture listener to detect SingleTap and DoubleTap in content (while leaving the chrome code path as-is).

This fixes the current bug, since it lets us send "click" events only for single-tap, and APZC is smart about delaying the single-tap notification until we know it's not part of a double-tap.

This isn't exactly ideal; in the long term I think we want to have fewer code paths, and move all of this mouse event simulation into cross-platform code (rather than widget or front-end code).  I'm open to suggestions for better near-term or long-term fixed.

Note: If we use this approach, we'll also want to fix bug 933990, and we should make APZC respect system settings for tap gestures if possible.
Attachment #8336992 - Flags: review?(jmathies)
Comment on attachment 8336984 [details] [diff] [review]
part 1: Don't send input to APZC if it hits chrome

Review of attachment 8336984 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/winrt/MetroInput.cpp
@@ +1138,5 @@
>    }
>  
>    // If this event is destined for chrome, deliver it directly there bypassing
>    // the apz.
> +  if (mChromeHitTestCacheForTouch) {

If skip over this when mCancelable=true we won't test the result of the first touchmove atrgetted at chrome. Hence chrome can't consume on that event. I'm not sure if this breaks anything in our current chrome script. I'd check input.js, selection, and the overlay button drag code to be sure. Down the line this might break extensions which expect is to adhere to the spec. Thoughts?
(In reply to Jim Mathies [:jimm] from comment #5)
> >    // If this event is destined for chrome, deliver it directly there bypassing
> >    // the apz.
> > +  if (mChromeHitTestCacheForTouch) {
> 
> If skip over this when mCancelable=true we won't test the result of the
> first touchmove atrgetted at chrome. Hence chrome can't consume on that
> event.

But we don't actually pay attention to whether chrome consumes touch events -- after the first touchmove, we *always* send these events to chrome and skip APZC.  So effectively, we always act like chrome "consumes" touch input whether it calls preventDefault or not.  This doesn't change any behavior; it just simplifies the code slightly and avoids uselessly sending these events to APZC.
Comment on attachment 8336984 [details] [diff] [review]
part 1: Don't send input to APZC if it hits chrome

Ok sounds good.
Attachment #8336984 - Flags: review?(jmathies) → review+
What's the delay set on the double tab detection? Seems pretty slow. For example, select some text, then tap once to clear it. There's a visual pause before the selection gets cleared. I also see this with link clicks. 

Can we tighten that up?
Attachment #8336992 - Flags: review?(jmathies) → review+
Generally not seeing any issues with this after testing for a bit.
(In reply to Jim Mathies [:jimm] from comment #8)
> What's the delay set on the double tab detection? Seems pretty slow. For
> example, select some text, then tap once to clear it. There's a visual pause
> before the selection gets cleared. I also see this with link clicks. 

Yeah, this is the downside of using double-tap as a gesture. :(

We currently wait MAX_TAP_TIME (300ms) to confirm a single-tap:
http://hg.mozilla.org/mozilla-central/file/09e33431c543/gfx/layers/ipc/GestureEventListener.cpp#l22
(In reply to Matt Brubeck (:mbrubeck) from comment #10)
> (In reply to Jim Mathies [:jimm] from comment #8)
> > What's the delay set on the double tab detection? Seems pretty slow. For
> > example, select some text, then tap once to clear it. There's a visual pause
> > before the selection gets cleared. I also see this with link clicks. 
> 
> Yeah, this is the downside of using double-tap as a gesture. :(
> 
> We currently wait MAX_TAP_TIME (300ms) to confirm a single-tap:
> http://hg.mozilla.org/mozilla-central/file/09e33431c543/gfx/layers/ipc/
> GestureEventListener.cpp#l22

I'd suggest we reduce that, especially with in-process content to about 100-150 msec.
https://hg.mozilla.org/mozilla-central/rev/31606d800f87
https://hg.mozilla.org/mozilla-central/rev/38dcab0d511c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Blocks: 941368
Depends on: 942689
Depends on: 943451
Blocks: 941774
Depends on: 950832
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: