[Webkit-unassigned] [Bug 143749] Force mouse events should go through normal mouse event handling code paths

[bugzilla-daemon at webkit.org]

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

--- Comment #6 from Beth Dakin <bdakin at apple.com> ---
(In reply to comment #3)
> Comment on attachment 250777 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250777&action=review
> 
> I would love to see some testing added in this page. I worry that if we
> don't add testing now, we'll never add it.
> 

We will definitely get to this! I'll start to investigate.

> > Source/WebCore/dom/Element.cpp:257
> > +    if (platformEvent.type() == PlatformEvent::MouseForceChanged && !document().hasListenerType(Document::FORCECHANGED_LISTENER))
> > +        return false;
> > +    if (platformEvent.type() == PlatformEvent::MouseForceDown && !document().hasListenerType(Document::FORCEDOWN_LISTENER))
> > +        return false;
> > +    if (platformEvent.type() == PlatformEvent::MouseForceUp && !document().hasListenerType(Document::FORCEUP_LISTENER))
> > +        return false;
> 
> A little helper that maps Document::*_LISTENER to PlatformEvent::Mouse*
> types would make this cleaner.
> 

I'm not sure I know what you mean. Are you just suggesting moving this to a helper function? A helper function that specifically maps LISTENER to PlatformEvent does not sound helpful to me, but lifting these three ifs into their own helper would be fine. I feel like I'm missing something here.

> > Source/WebCore/page/EventHandler.cpp:2116
> > +    if (m_frameSetBeingResized)
> > +        return !dispatchMouseEvent(eventNames().webkitmouseforcechangedEvent, m_frameSetBeingResized.get(), false, 0, event, false);
> 
> Weird. What's this about?
>
> > Source/WebCore/page/EventHandler.cpp:2135
> > +    if (m_frameSetBeingResized) {
> > +        dispatchMouseEvent(eventNames().webkitmouseforcechangedEvent, m_frameSetBeingResized.get(), false, 0, event, false);
> > +        return !dispatchMouseEvent(eventNames().webkitmouseforcedownEvent, m_frameSetBeingResized.get(), false, 0, event, false);
> > +    }
> 
> Same.
> 

There are lines one code like this when handling mousemoved and mouseup, which is why I added the code here. Turns out that whatever this concept is of framesets being resized is super ancient, and pre-dates EventHandler even being its own file. I could not find a sufficient explanation for what it does. Given that, and given that it is not consulted for other mouse events, I decided to remove these lines of code from the new force event handling functions.

> > Source/WebCore/page/EventHandler.cpp:2164
> > +bool EventHandler::handleMouseForceUpEvent(const PlatformMouseEvent& event)
> > +{
> > +    RefPtr<FrameView> protector(m_frame.view());
> > +
> > +    setLastKnownMousePosition(event);
> > +
> > +    if (m_frameSetBeingResized) {
> > +        dispatchMouseEvent(eventNames().webkitmouseforcechangedEvent, m_frameSetBeingResized.get(), false, 0, event, false);
> > +        return !dispatchMouseEvent(eventNames().webkitmouseforceupEvent, m_frameSetBeingResized.get(), false, 0, event, false);
> > +    }
> > +
> > +    HitTestRequest::HitTestRequestType hitType = HitTestRequest::DisallowShadowContent | HitTestRequest::Active;
> > +
> > +    HitTestRequest request(hitType);
> > +    MouseEventWithHitTestResults mouseEvent = prepareMouseEvent(request, event);
> > +
> > +    dispatchMouseEvent(eventNames().webkitmouseforcechangedEvent, mouseEvent.targetNode(), false, 0, event, false);
> > +    return !dispatchMouseEvent(eventNames().webkitmouseforceupEvent, mouseEvent.targetNode(), false, 0, event, false);
> > +}
> 
> Lots of very similar-looking code here. Can it be shared?
> 

Yes! Consolidated.

> > Source/WebCore/page/EventHandler.cpp:2178
> > +bool EventHandler::handleMouseForceChangedEvent(const PlatformMouseEvent& )
> > +{
> > +}
> > +
> > +bool EventHandler::handleMouseForceDownEvent(const PlatformMouseEvent&)
> > +{
> > +}
> > +
> > +bool EventHandler::handleMouseForceUpEvent(const PlatformMouseEvent&)
> > +{
> > +}
> 
> These need to return false.

Good point!

New patch coming soon.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150415/4098c42a/attachment.html>