[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

Dean Jackson <dino at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #250904|review?                     |review+
              Flags|                            |

--- Comment #13 from Dean Jackson <dino at apple.com> ---
Comment on attachment 250904
  --> https://bugs.webkit.org/attachment.cgi?id=250904
Patch with tests

View in context: https://bugs.webkit.org/attachment.cgi?id=250904&action=review

> Source/WebCore/page/EventHandler.cpp:2124
> +    bool swallowedEvent = !dispatchMouseEvent(eventNames().webkitmouseforcechangedEvent, mouseEvent.targetNode(), false, 0, event, false);
> +    if (event.type() == PlatformEvent::MouseForceDown)
> +        swallowedEvent = !dispatchMouseEvent(eventNames().webkitmouseforcedownEvent, mouseEvent.targetNode(), false, 0, event, false);
> +    if (event.type() == PlatformEvent::MouseForceUp)
> +        swallowedEvent = !dispatchMouseEvent(eventNames().webkitmouseforceupEvent, mouseEvent.targetNode(), false, 0, event, false);

Does this mean we can only swallow the event from forcechanged if down/up also swallow?

Technically I guess you could return from within both conditionals.

> Source/WebCore/replay/UserInputBridge.cpp:163
> +bool UserInputBridge::handleMouseForceChangedEvent(const PlatformMouseEvent& mouseEvent, InputSource)
> +{
> +    return m_page.mainFrame().eventHandler().handleMouseForceEvent(mouseEvent);
> +}
> +
> +bool UserInputBridge::handleMouseForceDownEvent(const PlatformMouseEvent& mouseEvent, InputSource)
> +{
> +    return m_page.mainFrame().eventHandler().handleMouseForceEvent(mouseEvent);
> +}
> +
> +bool UserInputBridge::handleMouseForceUpEvent(const PlatformMouseEvent& mouseEvent, InputSource)
> +{
> +    return m_page.mainFrame().eventHandler().handleMouseForceEvent(mouseEvent);
> +}

I haven't looked at how these are called, but is there any reason why they can't all be done in one handleMouseForceEvent? Do we need one for each type of input event name?

> Source/WebKit2/Shared/mac/WebEventFactory.mm:384
> +        if (lastPressureEvent.stage == 1 && event.stage == 2)
> +            type = WebEvent::MouseForceDown;
> +        else if (lastPressureEvent.stage == 2 && event.stage == 1)
> +            type = WebEvent::MouseForceUp;
> +        else

This is probably silly, but I wonder if we should have enums for the stage values so they read a bit better. Or are they actually just a count?

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:1912
> +        case PlatformEvent::MouseForceChanged:
> +            return page->corePage()->userInputBridge().handleMouseForceChangedEvent(platformMouseEvent);
> +        case PlatformEvent::MouseForceDown:
> +            return page->corePage()->userInputBridge().handleMouseForceDownEvent(platformMouseEvent);
> +        case PlatformEvent::MouseForceUp:
> +            return page->corePage()->userInputBridge().handleMouseForceUpEvent(platformMouseEvent);

Ooooh. Here's my answer to the other bit. So, considering these all do the same thing on the other end, do you think it makes sense to have a single handle function?

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:73
> +    self = [super init];
> +
> +    if (self) {
> +        _eventSender_location = location;
> +        _eventSender_locationInWindow = globalLocation; //????????
> +        _eventSender_stage = stage;
> +        _eventSender_pressure = pressure;
> +        _eventSender_phase = phase;
> +        _eventSender_timestamp = time;
> +        _eventSender_eventNumber = eventNumber;
> +    }
> +
> +    return self;

I know this isn't the general ObjC convention, but I was looking at Safari code yesterday that did this the other way around:

if (!self)
  return nil;

// other setup stuff

I guess we don't have coding style for that?

-- 
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/20150416/0767096a/attachment.html>