Chromium Code Reviews Chromium Code Reviews
Issue
404213003:
[WIP] Allow scroll events to permanently change the default gesture handler in RootView (Closed)
Description[WIP] Allow scroll events to permanently change the default gesture handler in RootView
(This CL is for discussion only, pending agreement I will
split this up into smaller CLs.)
If a view has been established as the default
gesture handler in RootView (the |gesture_handler_|
member) but it does not handle scroll events, then these
events will be sent to the closest ancestor of
|gesture_handler_| that does handle scroll events.
Currently, if any non-scroll gestures occur after the
scroll events but before the ui::ET_GESTURE_END, these
will be dispatched to the original |gesture_handler_|,
and not to the View that received the scroll events.
In this CL I propose the following change: If non-scroll
gestures occur after a scroll gesture but before the
ui::ET_GESTURE_END event (e.g., pinch), then these
events should be dispatched to the same View to which
the scrolls were dispatched. In other words, when the
default gesture handler in RootView is changed in
response to a scroll gesture, we never change it back
to its original value.
Reasons for making this change:
* The state maintained by RootView becomes simpler.
Instead of needing to store separate |gesture_handler_|
and |scroll_gesture_handler_| members, we will
only need to store a single |gesture_handler_| member.
* A side effect of the above is that the in-progress
removal of RootView::DispatchGestureEvent() becomes
simpler.
* The behavior is arguably more correct; if I am
currently scrolling a menu then I would expect
a pinch gesture to be dispatched to the menu itself,
not to the menu item I targeted on tap down.
With this change it will be possible to be
in an undesirable situation where the View
that received the ui::ET_GESTURE_BEGIN could be
different from the one that received the
ui::ET_GESTURE_END. To eliminate this problem:
* Ignore (do not dispatch) ui::ET_GESTURE_BEGIN to
any View. View subclasses which currently handle
GESTURE_BEGIN events should instead handle a more
specific event type such as
ui::ET_GESTURE_TAP_DOWN. All such cases can be
seen in this CL.
* Ignore (do not dispatch) ui::ET_GESTURE_END to any
View, but still clear the |gesture_handler_| member
upon seeing this event type. View subclasses
which currently handle GESTURE_END events should
instead handle a more specific event type
such as ui::ET_GESTURE_TAP_DOWN,
ui::ET_GESTURE_SCROLL_END, etc. All such cases
can be seen in this CL.
Added bonus: for each gesture sequence
(GESTURE_BEGIN, ..., GESTURE_END) we will have
to perform exactly two fewer dispatches and at most
two fewer event-targeting operations.
BUG=bugs to be filed for each smaller CL
TEST=TabDragControllerTest.DragWithGesture,
MenuButtonTest.ActivateNonDropDownOnGestureTap,
RootViewTest.ContextMenuFromLongPress,
WidgetTestInteractive.ResetCaptureOnGestureEnd,
WidgetTest.EventHandlersOnRootView,
WidgetTest.GestureBeginAndEndEvents,
WidgetTest.ScrollGestureEventDispatch (*)
(*) replaces ViewTest.ScrollGestureEvent
Patch Set 1 #Patch Set 2 : WIP #Patch Set 3 : added and changed some more tests #Patch Set 4 : compile error #Patch Set 5 : change to test name #Patch Set 6 : for initial feedback #Patch Set 7 : friend test #
Total comments: 18
Messages
Total messages: 12 (0 generated)
|
