.
<rdar://problem/76714308>
Created attachment 428577 [details] Patch
Comment on attachment 428577 [details] Patch This needs some tests.
Created attachment 429027 [details] Patch
Created attachment 429032 [details] Patch
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.
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.
Created attachment 429101 [details] Patch
Created attachment 429102 [details] Patch oops forgot test
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)
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].
*** Bug 145214 has been marked as a duplicate of this bug. ***
*** Bug 230545 has been marked as a duplicate of this bug. ***