Closed Bug 1286957 Opened 4 years ago Closed 4 years ago
_touch-action-inherit _child-pan-x-child-pan-y _touch-manual .html pass with APZ
On a windows touch device, the pointerevent_touch-action-inherit_child-pan-x-child-pan-y_touch-manual.html test fails. Note that in order to run this you have to do some fiddling with manifest files and such, it's not run in automation. The reason it fails is because the page is set up like so: <div style="overflow:scroll"> <div style="touch-action:pan-y"> <div style="touch-action:pan-x"> stuff </div> </div> </div> Because of the way we collection touch action regions in the code starting at , this gives us incorrect behaviour. Specifically, it marks the area as both pan-x and pan-y when in fact it should be neither.  http://searchfox.org/mozilla-central/rev/e269c68bc594b4a858624d73f0d9eb002f3c0844/layout/base/nsDisplayList.cpp#3678
Summary: Make pointerevent_touch-action-inherit_child-pan-x-child-pan-y_touch-manual.html pass → Make pointerevent_touch-action-inherit_child-pan-x-child-pan-y_touch-manual.html pass with APZ
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9229de2d992 includes this patch.
Comment on attachment 8772140 [details] [diff] [review] Patch We also combine these regions in FrameLayerBuilder, https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/layout/base/FrameLayerBuilder.cpp#3254 https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/layout/base/FrameLayerBuilder.cpp#3473 So wouldn't we need the same overlap logic there too?
Attachment #8772140 - Flags: review?(tnikkel) → review+
Yeah that's a good point. Also it looks like the combination of this patch and the patch on bug 1287142 causes a test failure (helper_touch_action_regions.html). The test passes with either patch individually, but when both are combined I guess the pan-y from the scrollframe overlaps with the pan-y from the scrolled frame and it turns into a d-t-c region, which causes the test to fail. I'll need to investigate a bit more though since that should only happen if the scrollframe hasn't been layerized and in that case it should already be in the d-t-c region.
Ah, looks like even after layerization, the scrolled frame gets added twice at  and . The first time would add the regions properly and the second time would convert it to d-t-c.  http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/layout/generic/nsFrame.cpp#2866  http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/layout/generic/nsFrame.cpp#2873
New try push, now with tests and more fixes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f60552da8f9
Now handling all of those other places you pointed out. Also unfortunately we have to fall back to d-t-c regions in even more cases than I originally thought.
Comment on attachment 8772993 [details] [diff] [review] Patch v2 Gross :)
Attachment #8772993 - Flags: review?(tnikkel) → review+
Attachment #8772994 - Flags: review?(tnikkel) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/e15ba82ad331 Because combining touch-action areas into the event regions is fraught with peril, dump them into the dispatch-to-content region. r=tnikkel https://hg.mozilla.org/integration/mozilla-inbound/rev/3d6cdeb53e16 Add some more complex mochitests for touch-action testing. r=tnikkel
You need to log in before you can comment on or make changes to this bug.