[Webkit-unassigned] [Bug 135195] Refactor EventHandler to call ScrollAnimator::handleWheelEvent for overflow scrolling

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jul 29 16:04:26 PDT 2014


https://bugs.webkit.org/show_bug.cgi?id=135195





--- Comment #28 from Beth Dakin <bdakin at apple.com>  2014-07-29 16:04:35 PST ---
(From update of attachment 235694)
View in context: https://bugs.webkit.org/attachment.cgi?id=235694&action=review

> Source/WebCore/page/EventHandler.cpp:290
> +    return scrollableArea->scroll(delta < 0 ? (isVerticalAxis ? ScrollUp : ScrollLeft) : (isVerticalAxis ? ScrollDown : ScrollRight), wheelGranularityToScrollGranularity(wheelEvent->deltaMode()), delta > 0 ? delta : -delta);

This is a little hard to read. Maybe you can pull some of those ternary operator evaluations out into well-named local variable?

>> Source/WebCore/page/EventHandler.cpp:293
>> +static inline bool handleWheelEventInAppropriateEnclosingBoxForSingleAxis(Node* startNode, WheelEvent* wheelEvent, Element** stopElement, bool isVerticalAxis)
> 
> I considered using an enum for this instead of a bool flag, but the only existing enum that made sense was ScrollbarOrientation, which sounded like it should be used for scrollbar-specific purposes. Meanwhile, ScrollDirection sounds more accurate, but it's specific to all 4 scrolling directions, whereas I only need the orientation (left/right vs. up/down).

I think you should consider adding a new enum. Something called ScrollGestureAxis or ScrollEventAxis or something along those lines?

> Source/WebCore/page/EventHandler.cpp:298
> +    RenderBox* initialEnclosingBox = &startNode->renderer()->enclosingBox();

Is there any reason you want to use this as a pointer instead of a reference?

> Source/WebCore/page/EventHandler.cpp:306
> +        if (boxLayer && (platformEvent != nullptr ? boxLayer->handleWheelEvent(isVerticalAxis ? platformEvent->copyIgnoringHorizontalDelta() : platformEvent->copyIgnoringVerticalDelta()) : didScrollInScrollableAreaForSingleAxis(boxLayer, wheelEvent, isVerticalAxis))) {

This is very difficult to read. Again, I suggest using some well-named local variables for the ternary operator evaluations.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list