Summary: | [iOS] Compatibility mouse events aren't prevented by calling preventDefault() on pointerdown | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||
Component: | UI Events | Assignee: | Antoine Quint <graouts> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cdumez, cmarcelo, dbates, dino, esprehn+autocc, ews-watchlist, kangil.han, thorton, webkit-bug-importer, zalan | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=198172 | ||||||||||
Attachments: |
|
Description
Antoine Quint
2019-05-22 09:24:32 PDT
Created attachment 370415 [details]
Patch
Attachment 370415 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/PlatformMouseEvent.h:65: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/PlatformMouseEvent.h:74: Wrong number of spaces before statement. (expected: 31) [whitespace/indent] [4]
Total errors found: 2 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 370417 [details]
Patch
Attachment 370417 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/PlatformMouseEvent.h:65: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/PlatformMouseEvent.h:74: Wrong number of spaces before statement. (expected: 31) [whitespace/indent] [4]
Total errors found: 2 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 370417 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370417&action=review > Source/WebKit/ChangeLog:376 > - * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp: Renamed from Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.cpp. > + * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp: Renamed from Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.pp. This seems wrong > Source/WebCore/page/PointerCaptureController.cpp:149 > +void PointerCaptureController::forgetPointer(PointerID pointerId) Forget! Interesting. Remove? DidRemove? Forget is fine too. > Source/WebCore/platform/PlatformMouseEvent.h:65 > - int clickCount, bool shiftKey, bool ctrlKey, bool altKey, bool metaKey, WallTime timestamp, double force, SyntheticClickType syntheticClickType) > + int clickCount, bool shiftKey, bool ctrlKey, bool altKey, bool metaKey, WallTime timestamp, double force, SyntheticClickType syntheticClickType, PointerID pointerId = mousePointerID()) This is getting ridiculous > Source/WebKit/UIProcess/ios/WKSyntheticTapGestureRecognizer.h:40 > +@property (nonatomic, weak) UIWebTouchEventsGestureRecognizer* supportingWebTouchEventsGestureRecognizer; > +@property (nonatomic, readonly) NSNumber* lastActiveTouchIdentifier; Stars are on the wrong side 🤷♂️ > Source/WebKit/UIProcess/ios/WKSyntheticTapGestureRecognizer.m:89 > + return; Should this just `break` out of the for() instead of returning from the function entirely? (just imagining someone coming along later and wanting to add code to touchesEnded) > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1040 > +void WebPage::forgetPointer(WebCore::PointerID pointerId) The name is still bothering me. Maybe it's the super generic "pointer". (In reply to Tim Horton from comment #6) > Comment on attachment 370417 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=370417&action=review > > > Source/WebKit/ChangeLog:376 > > - * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp: Renamed from Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.cpp. > > + * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp: Renamed from Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.pp. > > This seems wrong It is. Will fix in commit. > > Source/WebCore/page/PointerCaptureController.cpp:149 > > +void PointerCaptureController::forgetPointer(PointerID pointerId) > > Forget! Interesting. Remove? DidRemove? Forget is fine too. Instead of being so directive about the action to perform, I will update the method to be something more indicative about what happened and will rename to "touchWithIdentifierWasRemoved()". > > Source/WebKit/UIProcess/ios/WKSyntheticTapGestureRecognizer.h:40 > > +@property (nonatomic, weak) UIWebTouchEventsGestureRecognizer* supportingWebTouchEventsGestureRecognizer; > > +@property (nonatomic, readonly) NSNumber* lastActiveTouchIdentifier; > > Stars are on the wrong side 🤷♂️ Will fix in commit. > > Source/WebKit/UIProcess/ios/WKSyntheticTapGestureRecognizer.m:89 > > + return; > > Should this just `break` out of the for() instead of returning from the > function entirely? (just imagining someone coming along later and wanting to > add code to touchesEnded) Good point! Will fix in commit. > > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:1040 > > +void WebPage::forgetPointer(WebCore::PointerID pointerId) > > The name is still bothering me. Maybe it's the super generic "pointer". Yeah, see above, this is getting renamed to WebPage::touchWithIdentifierWasRemoved(). Comment on attachment 370417 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370417&action=review >>> Source/WebKit/ChangeLog:376 >>> + * NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp: Renamed from Source/WebKit/UIProcess/WebStorage/LocalStorageDatabaseTracker.pp. >> >> This seems wrong > > It is. Will fix in commit. We just announced Apple's new programming language! WWDC leak! >> Source/WebCore/platform/PlatformMouseEvent.h:65 >> + int clickCount, bool shiftKey, bool ctrlKey, bool altKey, bool metaKey, WallTime timestamp, double force, SyntheticClickType syntheticClickType, PointerID pointerId = mousePointerID()) > > This is getting ridiculous Yeah, can we make a PlatformMouseEventInfo or Data? Which would be a subclass of PlatformEventInfo? > Source/WebCore/platform/PointerID.h:32 > +inline PointerID mousePointerID() { return 1; } Why is this a function rather than a static PointerID mousePointerID = 1; ? (In reply to Dean Jackson from comment #8) > Comment on attachment 370417 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=370417&action=review > > > Source/WebCore/platform/PointerID.h:32 > > +inline PointerID mousePointerID() { return 1; } > > Why is this a function rather than a static PointerID mousePointerID = 1; ? Will fix in commit. Committed r245639: <https://trac.webkit.org/changeset/245639> This broke the tvOS build, see https://bugs.webkit.org/show_bug.cgi?id=198172. This was fixed in https://trac.webkit.org/changeset/245673. Reopening, this doesn't work when the synthetic tap is delayed due to double-tap-to-zoom. So the patch worked fine on iPadOS but not with iOS on an iPhone. Created attachment 372625 [details]
Patch
Committed r246693: <https://trac.webkit.org/changeset/246693> |