575440 - Deal with eWindowType_toplevel on WM_GESTURENOTIFY for bug 130078 landing

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

on widget/src/windows/nsWindow.cpp we have:

5159 case WM_GESTURENOTIFY:
5160 {
5161 if (mWindowType != eWindowType_invisible &&
5162 mWindowType != eWindowType_plugin &&
5163 mWindowType != eWindowType_toplevel) {
5164 // eWindowType_toplevel is the top level main frame window. Gesture support
5165 // there prevents the user from interacting with the title bar or nc
5166 // areas using a single finger. Java and plugin windows can make their
5167 // own calls.
5168 GESTURENOTIFYSTRUCT * gestureinfo = (GESTURENOTIFYSTRUCT*)lParam;
5169 nsPointWin touchPoint; 
5170 ...

The window type of the main content area will change with bug 130078 (I guess it's gonna be eWindowType_toplevel, right?)

Timothy: I'm pretty sure that check and comment are outdated now and we can just remove it. When I wrote that handling this check already existed, I just moved it around. The gesturenotify event that is dispatched probably cover everything needed to remove the check.

I'll verify it and let you know.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: Remove eWindowType_topLevel check

Yeah, the eWindowType_topLevel check can be removed from there. I test this patch + patches from bug 130078 and all panning + touch works.

Jim, originally we skipped eWindowType_topLevel for WM_GESTURENOTIFY because we only activated single-finger panning unconditionally on content windows. After we added the nsGestureNotify event to find scrollable areas, that check is no longer necessary because we're able to better choose how to activate gestures. This check should go now, otherwise this will always be ignored after bug 130078 lands.

Comment 2

[Timothy Nikkel (:tnikkel)]

Has this been try server-ed? I can land this, just need to know if it needs try server or not.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

No need to tryserver this, there isn't anything that this would affect on a try run. I was planning to land this the next time I land other patches (tomorrow or monday), but if you end up landing 130078 first feel free to take this patch and land it together.

Comment 4

[:Felipe Gomes (needinfo for replies!)]

http://hg.mozilla.org/mozilla-central/rev/581b8030bc1a