[Webkit-unassigned] [Bug 135476] Implement long mouse press over links. Part of 135257 - Add long mouse press gesture.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Aug 8 14:32:07 PDT 2014
https://bugs.webkit.org/show_bug.cgi?id=135476
Peyton Randolph <prandolph at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Blocks|135767 |
--- Comment #10 from Peyton Randolph <prandolph at apple.com> 2014-08-08 14:32:17 PST ---
(In reply to comment #9)
> (From update of attachment 236247 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236247&action=review
>
> > Source/WebCore/page/EventHandler.cpp:166
> > +#if ENABLE(LONG_MOUSE_PRESS)
>
> Simon noted that you might want to get rid of these ENABLEs, since there's nothing particularly scary or platform-specific about this feature, and it has/will have a runtime flag, and won't do anything if the client doesn't care, anyway. That said, I think that can happen later, perhaps in https://bugs.webkit.org/show_bug.cgi?id=135721.
I agree with doing so later. I wasn't up-to-date on WebKit conventions when I added the flag. Ideally the runtime switch (https://bugs.webkit.org/show_bug.cgi?id=135682) would have come first.
>
> > Source/WebCore/page/EventHandler.cpp:168
> > +const int longMousePressPointsHysteresis = 5;
>
> Maybe "maximumDistanceForLongMousePress", or something (that's not a great name either, but a little better). "pointsHysteresis" reads oddly to me.
I've renamed to MaximumLongMousePressDragDistance and added a comment that its units are in points.
>
> > Source/WebCore/page/EventHandler.cpp:780
> > + beginLongMousePress();
>
> Is this "beginTrackingPotentialLongMousePress"? because I don't think the *long* mouse press is technically beginning yet. But I dunno!
The term is borrowed from UIKit's gesture recognizers. It means "begin determining if a long mouse press is occurring."
I like your interpretation though. I will rename this and cancelLongMousePress to cancelTrackingPotentialLongMousePress as well.
>
> > Source/WebCore/page/EventHandler.cpp:1585
> > + // FIXME: bubble long mouse press up to the client.
>
> Capital 'B', perhaps. Also, do you have a bug number for these?
Capitalized and added bug number, https://bugs.webkit.org/show_bug.cgi?id=135580
>
> > Source/WebCore/page/EventHandler.cpp:1619
> > + if (mouseEvent.button() != LeftButton || mouseEvent.type() != PlatformEvent::MouseMoved)
>
> Why would long press be restricted to the left mouse button? Should we not support long-press of any arbitrary button and let the client decide which ones it cares about? I could see that being useful.
Ah, good idea. For this patch, I would like to restrict ourselves to left mouse presses because it's difficult enough test the single case of left-mouse-pressing links. I filed a bug tracking the extension to arbitrary mouse clicks: https://bugs.webkit.org/show_bug.cgi?id=135767.
>
> > Source/WebCore/page/EventHandler.cpp:1630
> > + if (!mouseHysteresisExceeded(mouseEvent.position(), longMousePressPointsHysteresis))
> > + return false;
> > +
> > + cancelLongMousePress();
> > +
> > + return false;
>
> I think this would read better as just:
>
> if (mouseHysteresisExceeded(mouseEvent.position(), longMousePressPointsHysteresis))
> cancelLongMousePress();
>
> return false;
>
Done.
> > Source/WebCore/page/EventHandler.cpp:3527
> > +bool EventHandler::mouseHysteresisExceeded(const FloatPoint& viewportLocation, const int pointsThreshold) const
>
> This is still technically about drag, in some sense. Maybe "dragDistanceExceedsThreshold"? That sounds good in an if statement ("if dragDistanceExceedsThreshold").
While dragging requires a mouse button to be pressed, this method does not. Maybe mouseMovementExceedsThreshold?
>
> > Source/WebCore/page/EventHandler.cpp:3535
> > + return abs(delta.width()) >= pointsThreshold || abs(delta.height()) >= pointsThreshold;
>
> Should this be something like delta.diagonalLengthSquared() >= pointsThreshold * pointsThreshold, to make a circle of hysteresis?
>
> I see you got this from existing code, so maybe don't change it, but I wonder why!
I agree, the box model is pretty weird :/ I'm going to leave it as-is since this code is used by dragging, so I don't know if there's something else counting on this behavior.
--
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