Closed Bug 1144324 Opened 7 years ago Closed 7 years ago

Do a better job of registering for touch events in the windows widget code


(Core :: Widget: Win32, defect)

Windows 8
Not set



Tracking Status
firefox39 --- fixed


(Reporter: kats, Assigned: kats)




(2 files)

So right now we have the funky code at [1] which causes the widget to register as a "touch window" when there are touch listeners attached and unregister when there are no touch listeners. This is was supposedly a half-decent solution for the state of things back in the day, but it was a very "magical" behavior which has led to much angst where scrolling was broken on pages with touch listeners (see bug 736048). This behaviour was eventually disabled by setting the dom.w3c_touch_events.enabled pref to 0 on all desktop firefoxen.

I think this code has outlived its usefulness now that we have APZ, and should just be killed. APZ isn't yet enabled by default on desktop, so we still need to keep the existing touch-based scrolling code in in widget/windows. However, we can do a better job of picking between that and APZ on startup and then just sticking with one scrolling implementation. I have patches that do this and they seem to work as expected.
This makes it so that if the window gets an APZCTreeManager (i.e. we have layers.async-pan-zoom.enabled set to true and the window is a top-level or child window), then we automatically try to register as a touch window. The registration itself further checks for the dom.w3c_touch_events.enabled and dom.w3c_pointer_events.enabled prefs, and only succeeds if one or both of those is true. Therefore, setting both layers.async-pan-zoom.enabled and dom.w3c_touch_events.enabled will switch desktop windows to scroll using APZ with touch events.
Attachment #8578864 - Flags: review?(jmathies)
Attachment #8578864 - Flags: review?(dvander)
And this removes all the machinery that we don't need any more to track if have touch listeners or not.
Attachment #8578866 - Flags: review?(jmathies)
Attachment #8578866 - Flags: review?(bugs)
In my local testing I was working on top of the patches in these two bugs. There shouldn't be an actual conceptual dependency but I didn't test these patches in isolation.
Depends on: 1144112, 1122090
Attachment #8578864 - Flags: review?(dvander) → review+
Comment on attachment 8578866 [details] [diff] [review]
Part 2 - Remove all unneeded machinery

Update IID of nsPIDOMWindow.

I thought the original reason for this code was touch vs. gesture events case on
Windows 7. But if we can remove this, fine.
Attachment #8578866 - Flags: review?(bugs) → review+
That try push got screwed up by infra melting. Did another push:
Attachment #8578866 - Flags: review?(jmathies) → review+
Attachment #8578864 - Flags: review?(jmathies) → review+
Note that the second patch here broke some stuff on Fennec, so I backed out part of it to fix bug 1146843. We still need to track the touch listeners (although we don't need the UpdateTouchWindow goop), until Fennec switches over to the C++ APZ code. Then we can rip it out again.
You need to log in before you can comment on or make changes to this bug.