Optimizations to remove 300ms touch > mouse events delay

RESOLVED FIXED in mozilla27



6 years ago
6 years ago


(Reporter: redux, Assigned: daleharvey)



Dependency tree / graph

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [c= p= s= u=])


(3 attachments, 2 obsolete attachments)



6 years ago
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/29.0.1547.76 Safari/537.36

Steps to reproduce:

1) go to http://patrickhlauke.github.io/touch/tests/event-listener_user-scalable-no.html and tap the button
2) go to http://patrickhlauke.github.io/touch/tests/event-listener_minimum-maximum-scale.html and tap the button

Actual results:

in both cases, the ~300ms delay between the touch events and the simulated/fallback mouse events is present (generally present in touch-devices to wait for a potential second tap to initiate a double-tap-to-zoom)

Expected results:

as is already the case in Firefox for Android, the delay should not be necessary in situations where a page has been explicitly set not to scale/zoom - classic examples are having the viewport meta set to user-scalable=no or having the minimum and maximum scale set to the same value.
Keywords: perf

Comment 1

6 years ago
worth noting that i'm filing this bug indirectly - don't have a real FirefoxOS device, but screenshots were kindly provided for me by a friend running on a ZTE (which may not be the latest version)
Confirmed with FxOS 1.3 so assuming that it is there for previous versions.

Gecko git sha is dbb784a
Ever confirmed: true
Whiteboard: [c= p= s= u=]
Seems like this is probably a gecko issue, not gaia.  Kind of guessing at the component here.
Component: General → DOM: Events
Product: Boot2Gecko → Core
This is mostly in the gfx/layers async pan/zoom code:
Component: DOM: Events → Graphics: Layers
Yeah, it's in the APZC code. In Fennec the JavaPanZoomController handles this by firing the tap event early if it knows it doesn't need to wait for a double-tap because the page is not zoomable.


We should put similar code into the APZC.
Dale, would you mind taking this one since you're working on that code anyway?
Flags: needinfo?(dale)

Comment 7

6 years ago
Sure thing, cheers
Assignee: nobody → dale
Flags: needinfo?(dale)

Comment 9

6 years ago
This fixes the delay on the mouseover event being fired, I am still seeing a 300ms delay on mousemove only in http://patrickhlauke.github.io/touch/tests/event-listener_minimum-maximum-scale.html with this patch
Comment on attachment 814370 [details] [diff] [review]
Fire single tap immediately if zooming disabled

Review of attachment 814370 [details] [diff] [review]:

I don't like the approach of changing the gesture detector itself based on whether the APZC is zoomable. I would prefer mirroring the approach we use in Fennec, where APZC::OnSingleTapUp fires controller->HandleSingleTap if the page is not zoomable and APZC::OnSingleTapConfirmed fires it if it is zoomable.
Attachment #814370 - Flags: review?(bugmail.mozilla) → review-

Comment 11

6 years ago
That is cleaner

I wasnt sure about the code duplication / whether its worth splitting into a new function or not, can do that if needed
Attachment #814370 - Attachment is obsolete: true
Attachment #815595 - Flags: review?(bugmail.mozilla)
Comment on attachment 815595 [details] [diff] [review]
Fire tap immediately if content is not zoomable

Review of attachment 815595 [details] [diff] [review]:

Looks good, thanks!
Attachment #815595 - Flags: review?(bugmail.mozilla) → review+

Comment 13

6 years ago
Rebased, carrying r+, inbound is closed so will push when its reopened
Attachment #815595 - Attachment is obsolete: true
Attachment #816405 - Flags: review+
Vivien - Just wanted to bring this bug to your attention as I believe you were thinking of relying on the mouse delay for styling selected elements with :hover in FirefoxOS. I'm not sure how or if this will impact your plans though.
Flags: needinfo?(21)

Comment 15

6 years ago
This only affects the delay after touchup, seems like :hover should apply on touchdown

Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27

Comment 17

6 years ago
line 717	  if (controller && mAllowZoom) {
line 717	  if (controller && !mAllowZoom) {

Comment 18

6 years ago
Apologies, screwed that up during the rebase, will fix now

Comment 19

6 years ago
Follow up to fix error in rebase

Comment 21

6 years ago
Dale, any reason we wouldn't want this for koi (1.2)?
Flags: needinfo?(dale)
Whiteboard: [c= p= s= u=] → [c= p= s=2013.10.18 u=]

Comment 22

6 years ago
Its not a bug fix and there is going to be a lot of changes and improvements in this area, I would prefer we only keep uplifts for 'must fixes' and try to complete old release asap, couldnt really make a call on this bug over the others though, https://bugzilla.mozilla.org/show_bug.cgi?id=915645 has leo nom and so will likely conflict if this isnt uploaded
Flags: needinfo?(dale)


6 years ago
Whiteboard: [c= p= s=2013.10.18 u=] → [c= p= s= u=]
(In reply to Dale Harvey (:daleharvey) PTO - 28th Oct from comment #15)
> This only affects the delay after touchup, seems like :hover should apply on
> touchdown
> https://hg.mozilla.org/integration/mozilla-inbound/rev/d4e5777ad39d

:hover per spec applies on mousemove iirc. I have introduced a delay to have :active working though but this stuff affects BrowserElementPanning.js which is a code path that is not taken by this code so it should be fine. But I bet that at some point we will have to introduce an artificial delay in this code as well in order to have :active working properly.
Flags: needinfo?(21)
Blocks: 944082
You need to log in before you can comment on or make changes to this bug.