Bug 225788 – Add a way to create `"wheel"` events from gesture/touch events

• Component: New Bugs
• Status: RESOLVED FIXED

[Devin Rousso]

.

[Devin Rousso]

<rdar://problem/76714308>

[Devin Rousso]

Created attachment 428577 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 428577 [details]
Patch

This needs some tests.

[Devin Rousso]

Created attachment 429027 [details]
Patch

[Devin Rousso]

Created attachment 429032 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 429032 [details]
Patch

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

> Source/WebCore/platform/PlatformWheelEvent.cpp:49
> +    platformWheelEvent.m_timestamp = platformGestureEvent.timestamp();
> +    platformWheelEvent.m_modifiers = platformGestureEvent.modifiers() | PlatformEvent::Modifier::ControlKey;

Can we give PlatformWheelEvent a PlatformEvent copy ctor that initializes the shared members? Otherwise someone will add something in future and not know to fix this code path.

> Source/WebCore/platform/PlatformWheelEvent.cpp:61
> +        platformWheelEvent.m_phase = deltaY ? PlatformWheelEventPhase::Began : PlatformWheelEventPhase::MayBegin;

This only makes sense if the first event has deltaY=0 and no subsequent event does (MayBegin should only ever be followed by Began or Canceled).

> Source/WebCore/platform/PlatformWheelEvent.cpp:62
> +        platformWheelEvent.m_momentumPhase = deltaY ? PlatformWheelEventPhase::Began : PlatformWheelEventPhase::None;

It doesn't make sense to have an event where both m_phase and m_momentumPhase are PlatformWheelEventPhase::Began. m_phase and m_momentumPhase are really mutually exclusive. There are no momentum phases in this gesture, I think.

> Tools/ChangeLog:42
> +        Make sure to pass all feature flags when generating JS files from IDL files.

Oh finally! But what about DumpRenderTree?

> Tools/WebKitTestRunner/DerivedSources.make:43
> +FRAMEWORK_FLAGS := $(shell echo $(BUILT_PRODUCTS_DIR) $(FRAMEWORK_SEARCH_PATHS) $(SYSTEM_FRAMEWORK_SEARCH_PATHS) | $(PERL) -e 'print "-F " . join(" -F ", split(" ", <>));')
> +HEADER_FLAGS := $(shell echo $(BUILT_PRODUCTS_DIR) $(HEADER_SEARCH_PATHS) $(SYSTEM_HEADER_SEARCH_PATHS) | $(PERL) -e 'print "-I" . join(" -I", split(" ", <>));')
> +FEATURE_AND_PLATFORM_DEFINES := $(shell $(CC) -std=gnu++1z -x c++ -E -P -dM $(SDK_FLAGS) $(TARGET_TRIPLE_FLAGS) $(FRAMEWORK_FLAGS) $(HEADER_FLAGS) -include "wtf/Platform.h" /dev/null | $(PERL) -ne "print if s/\#define ((HAVE_|USE_|ENABLE_|WTF_PLATFORM_)\w+) 1/\1/")

Someone else should review this.

> Tools/WebKitTestRunner/EventSenderProxy.h:94
> +    void scaleGestureStart(double scale);
> +    void scaleGestureChange(double scale);
> +    void scaleGestureEnd(double scale);

Don't these have an origin too?

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:133
> +    CGEventSetIntegerValueField(cgEvent.get(), kCGEventGestureHIDType, 8 /* kIOHIDEventTypeZoom */);

Should we put kIOHIDEventTypeZoom in an SPI header?

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:910
> +    auto event = adoptNS([[EventSenderSyntheticEvent alloc] initMagnifyEventAtLocation:NSMakePoint(m_position.x, m_position.y)

Maybe pull [m_testController->mainWebView()->platformView() into a local variable.

[Devin Rousso]

Comment on attachment 429032 [details]
Patch

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

>> Source/WebCore/platform/PlatformWheelEvent.cpp:62
>> +        platformWheelEvent.m_momentumPhase = deltaY ? PlatformWheelEventPhase::Began : PlatformWheelEventPhase::None;
> 
> It doesn't make sense to have an event where both m_phase and m_momentumPhase are PlatformWheelEventPhase::Began. m_phase and m_momentumPhase are really mutually exclusive. There are no momentum phases in this gesture, I think.

Ah good catch.  I misread `momentumPhaseForEvent` as using `[event phase]` not `[event momentumPhase]`.  I'll double check this.

>> Tools/ChangeLog:42
>> +        Make sure to pass all feature flags when generating JS files from IDL files.
> 
> Oh finally! But what about DumpRenderTree?

I didn't see any IDL files in `DumpRenderTree`.  I can add it though.

>> Tools/WebKitTestRunner/DerivedSources.make:43
>> +FEATURE_AND_PLATFORM_DEFINES := $(shell $(CC) -std=gnu++1z -x c++ -E -P -dM $(SDK_FLAGS) $(TARGET_TRIPLE_FLAGS) $(FRAMEWORK_FLAGS) $(HEADER_FLAGS) -include "wtf/Platform.h" /dev/null | $(PERL) -ne "print if s/\#define ((HAVE_|USE_|ENABLE_|WTF_PLATFORM_)\w+) 1/\1/")
> 
> Someone else should review this.

FYI this is a copypasta from `WebCore/DerivedSources.make`.

>> Tools/WebKitTestRunner/EventSenderProxy.h:94
>> +    void scaleGestureEnd(double scale);
> 
> Don't these have an origin too?

The most recent mouse location is the origin.  I was matching how force events are simulated in that it requires a `moveMouseTo` beforehand.

>> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:133
>> +    CGEventSetIntegerValueField(cgEvent.get(), kCGEventGestureHIDType, 8 /* kIOHIDEventTypeZoom */);
> 
> Should we put kIOHIDEventTypeZoom in an SPI header?

It's actually already in `Source/WebCore/PAL/pal/spi/cocoa/IOKitSPI.h`.  I'll just include it in this file.

[Devin Rousso]

Created attachment 429101 [details]
Patch

[Devin Rousso]

Created attachment 429102 [details]
Patch

oops forgot test

[Devin Rousso]

Comment on attachment 429102 [details]
Patch

test failures are probably unrelated as this patch doesn't actually have an functional changes (other than to how the test runner is compiled)

[EWS]

Committed r277772 (237933@main): <https://commits.webkit.org/237933@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 429102 [details].

[Tim Horton]

*** Bug 145214 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

*** Bug 230545 has been marked as a duplicate of this bug. ***