<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Force mouse events should go through normal mouse event handling code paths"
href="https://bugs.webkit.org/show_bug.cgi?id=143749#c6">Comment # 6</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Force mouse events should go through normal mouse event handling code paths"
href="https://bugs.webkit.org/show_bug.cgi?id=143749">bug 143749</a>
from <span class="vcard"><a class="email" href="mailto:bdakin@apple.com" title="Beth Dakin <bdakin@apple.com>"> <span class="fn">Beth Dakin</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=143749#c3">comment #3</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=250777&action=diff" name="attach_250777" title="Patch">attachment 250777</a> <a href="attachment.cgi?id=250777&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=250777&action=review">https://bugs.webkit.org/attachment.cgi?id=250777&action=review</a>
>
> 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.
> </span >
We will definitely get to this! I'll start to investigate.
<span class="quote">> > 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.
> </span >
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.
<span class="quote">> > 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?</span >
>
<span class="quote">> > 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.
> </span >
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.
<span class="quote">> > 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?
> </span >
Yes! Consolidated.
<span class="quote">> > 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.</span >
Good point!
New patch coming soon.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>