STR (1) Load nytimes.com, wait to finish (2) Try to fling down the page (3) Try to fling down the page again You'll most likely see either (2) succeed and (3) fail, or (2) fail and (3) succeed. It seems like the gesture detection in APZC is being messed up every other gesture. This takes perceived browser performance to crap, and as a regression should block. Shih-Chiang do you have cycles to take a look?

@cjones, sure, I'll take a look at this bug.

APZC delays gesture detection because www.nytimes.com has touch event listener. If we have more than one touch transaction in queue, APZC will only process the first touch transaction in queue. Therefore, you'll one see one gesture been triggered.

(In reply to Shih-Chiang Chien [:schien] from comment #2) > only process the first touch transaction in queue. Therefore, you'll one see s/one see/only see

Comment on attachment 699737 [details] [diff] [review] WIP - process all queued touch transactions after content process finishing event handler. This seems right to me. I reviewed the original patch but I don't remember this check specifically. drs, do you recall why this is here?

Comment on attachment 699737 [details] [diff] [review] WIP - process all queued touch transactions after content process finishing event handler. The idea behind this code block was that we should require a preventDefault or no-preventDefault for each block of touch events from start->n*move->end/cancel. Removing this block will make us process all queued up touch events. That means that if we touch the screen 3 independent times (i.e. I put my finger down, do some arbitrary motion, then take my finger off) before content manages to handle one of them, the first one to handle it will cause the following ones to be processed at the same time. We're already not following the W3 spec for this, but here's the important part of what it has to say about this (http://www.w3.org/TR/touch-events/#list-of-touchevent-types): > If the preventDefault method is called on this event, it should prevent any > default actions caused by any touch events associated with the same active > touch point, including mouse events or scrolling. My interpretation of this is that we should be associating each preventDefault/no-preventDefault to the series of touch events that it was handling (start->n*move->end/cancel). Removing this code would break that. The reason this isn't working is because, if touch events with different mIdentifier fields are interspersed, only the first touchend will get sent. I'd rather fix this another way like batching touch events into queues based on the mIdentifier field rather than chronological ordering, but that seems like a lot of work and personally I'm okay with fixing it this way for now as long as we come back to it (i.e. file tech debt bugs).

add FIXME comment.

Comment on attachment 700344 [details] [diff] [review] process all queued touch transactions after content process finishing event handler, v1 This patch doesn't fix the issue for me :(. I suspect this is related somehow to bug 828367 and async-pan-zoom detection.

(In reply to Shih-Chiang Chien [:schien] from comment #6) > Created attachment 700344 [details] [diff] [review] > process all queued touch transactions after content process finishing event > handler, v1 > > add FIXME comment. If this patch ever sees the light of day, I'd prefer it if (a) bug(s) were filed for this instead of just adding a FIXME.

schien, do you want to keep working on this?

(In reply to Andreas Gal :gal from comment #9) > schien, do you want to keep working on this? I handed it over to :nrc yesterday, and I'll help him on this bug since I have bandwidth now. :)

I find this difficult to recreate with flings, but pretty easy with pinch-zooms. I don't think that the issue is with prematurely clearing the queue though, we don't ever hit the path changed in the patch when we miss the gestures. Investigating...

We're not entirely sure this is safe behaviour, but it certainly fixes the dropped pinch-to-zoom gestures. We still need to fix the fling issue.

Patch is joint work with schien, btw. Teamwork FTW!

Could someone land this to b2g18 for me please? I seem to be having hg issues with that repo :-(

https://hg.mozilla.org/releases/mozilla-b2g18/rev/8f2ef4998b60 (Mind the [leave open], please.)

Kats, have you been able to reproduce the fling issue?

(In reply to JP Rosevear [:jpr] from comment #17) > Kats, have you been able to reproduce the fling issue? No, I can't reproduce it either on my device (test-drivers device running the latest available build) or on a B2G Desktop gaia build.

The problem is that when the time between samples gets 'large', the velocity gets set to 0 too quickly because the way we calculate the new velocity from the friction is incorrect. I changed the way it is calculated and that fixes the bug. I adjusted the friction global to give apx the same speed of flinging.

Chris, if this gets an r+, could you land it as well? Nick is in GMT, and we want to land this today.

https://hg.mozilla.org/integration/mozilla-inbound/rev/66c886b7d927 https://hg.mozilla.org/releases/mozilla-b2g18/rev/f5278c18083b

backed out of inbound due to build bustage https://hg.mozilla.org/integration/mozilla-inbound/rev/66c886b7d927 I don't have the b2g18 repo here to do the same there... please take care of that.

the failure on Windows e:/builds/moz2_slave/m-in-w32/build/gfx/layers/ipc/Axis.cpp(165) : error C2666: 'pow' : 6 overloads have similar conversions

ehm sorry, I posted the wrong backout changeset: https://hg.mozilla.org/integration/mozilla-inbound/rev/d94202f46627

gecko18 relanding https://hg.mozilla.org/releases/mozilla-b2g18/rev/e9eca4f2ea0f inbound coming up when it reopens.

Comment on attachment 701112 [details] [diff] [review] part 1: fix pinch to zoom behaviour Review of attachment 701112 [details] [diff] [review]: ----------------------------------------------------------------- Clearing from my review queue, also f+ FWIW at this point.

Comment on attachment 701844 [details] [diff] [review] part 2: fix flings Review of attachment 701844 [details] [diff] [review]: ----------------------------------------------------------------- Clearing from my review queue, also f+ FWIW at this point.