Cancel Web Touches Properly
Created attachment 359780 [details] Patch
Comment on attachment 359780 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359780&action=review > Source/WebKit/ChangeLog:3 > + Cancel Web Touches Properly Can(cel) This Bug & Patch Have A Title That Describes The Symptom? > Source/WebKit/ChangeLog:5 > + And Probably A Radar #?
Created attachment 359786 [details] Patch
Comment on attachment 359786 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359786&action=review > Source/WebKit/Platform/spi/ios/UIKitSPI.h:761 > +- (void)cancel; Was this here forever into the past, or do you need some ifdefs? > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1354 > +- (void)_cancelWebGestureRecognizer > +{ > + [_touchEventGestureRecognizer cancel]; > +} Why the indirection? Why "Web" gesture recognizer? Aren't they all technically about the web? :D I could see _cancelTouchEventsGestureRecognizer or something, but also think you just don't need this function.
Created attachment 359791 [details] Patch
Comment on attachment 359791 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359791&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1351 > +- (void)_cancelWebGestureRecognizer Still not sure why the method. Are you thinking it will have more callers? And my other complaint about the method name stills stands. > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1353 > +#if HAVE(CANCEL_WEB_TOUCH_EVENTS_GESTURE) I think the name I provided before was closer to the other names in e.g. Platform.h, but I guess this is OK.
Comment on attachment 359791 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359791&action=review >> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1351 >> +- (void)_cancelWebGestureRecognizer > > Still not sure why the method. Are you thinking it will have more callers? > And my other complaint about the method name stills stands. They are this is the Web Gesture in all other instances, because it is the gesture that passes the touches to the web. I think this helps clear up that is this the gesture we are talking about. And while there are no additional callers today, there might be in the future, so I'd like to encapsulate the need for feature defines. Also, several other gestures use this exact layout of code to do the exact same thing.
Comment on attachment 359791 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359791&action=review >>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1351 >>> +- (void)_cancelWebGestureRecognizer >> >> Still not sure why the method. Are you thinking it will have more callers? >> And my other complaint about the method name stills stands. > > They are this is the Web Gesture in all other instances, because it is the gesture that passes the touches to the web. I think this helps clear up that is this the gesture we are talking about. > And while there are no additional callers today, there might be in the future, so I'd like to encapsulate the need for feature defines. Also, several other gestures use this exact layout of code to do the exact same thing. I don't think it's ever called just "Web", that would be crazy. It is "WebTouchEvent" in UIKit, and "TouchEvent" in WebKit. You can decide what to call it.
Created attachment 359825 [details] Patch for landing
<rdar://problem/47056717>
Comment on attachment 359825 [details] Patch for landing Clearing flags on attachment: 359825 Committed r240316: <https://trac.webkit.org/changeset/240316>
All reviewed patches have been landed. Closing bug.