UIGestureRecognizer SPI is guarded by an iOS macro, but we need the definition for watchOS and tvOS.
<rdar://problem/64217654>
Created attachment 401561 [details] Patch
Comment on attachment 401561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401561&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1457 > + return [recognizer respondsToSelector:@selector(modifierFlags)] ? [recognizer modifierFlags] : 0; This gets watchOS and tvOS building, but I'm pretty sure it's wrong.
Comment on attachment 401561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401561&action=review >> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1457 >> + return [recognizer respondsToSelector:@selector(modifierFlags)] ? [recognizer modifierFlags] : 0; > > This gets watchOS and tvOS building, but I'm pretty sure it's wrong. Let's not commit something that we are pretty sure is wrong.
Comment on attachment 401561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401561&action=review >>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1457 >>> + return [recognizer respondsToSelector:@selector(modifierFlags)] ? [recognizer modifierFlags] : 0; >> >> This gets watchOS and tvOS building, but I'm pretty sure it's wrong. > > Let's not commit something that we are pretty sure is wrong. Jonathan, don't you just want to make UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS be true on watchOS and tvOS?
(In reply to Tim Horton from comment #5) > Comment on attachment 401561 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401561&action=review > > >>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1457 > >>> + return [recognizer respondsToSelector:@selector(modifierFlags)] ? [recognizer modifierFlags] : 0; > >> > >> This gets watchOS and tvOS building, but I'm pretty sure it's wrong. > > > > Let's not commit something that we are pretty sure is wrong. > > Jonathan, don't you just want to make UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS > be true on watchOS and tvOS? err, HAVE()
Comment on attachment 401561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401561&action=review >>>>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1457 >>>>> + return [recognizer respondsToSelector:@selector(modifierFlags)] ? [recognizer modifierFlags] : 0; >>>> >>>> This gets watchOS and tvOS building, but I'm pretty sure it's wrong. >>> >>> Let's not commit something that we are pretty sure is wrong. >> >> Jonathan, don't you just want to make UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS be true on watchOS and tvOS? > > err, HAVE() Maybe? Shouldn't the definition of modifierFlags also be guarded by HAVE(UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS), in that case? I'm a bit confused by the original content of this line in the first place. It seems like it would only compile if the recognizer had a _modifierFlags member variable, which it doesn't seem to have for watchOS. I would expect this code to read: #if HAVE(UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS) return recognizer.modifierFlags; #else return 0; #endif
(In reply to Jonathan Bedard from comment #7) > Comment on attachment 401561 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401561&action=review > > >>>>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1457 > >>>>> + return [recognizer respondsToSelector:@selector(modifierFlags)] ? [recognizer modifierFlags] : 0; > >>>> > >>>> This gets watchOS and tvOS building, but I'm pretty sure it's wrong. > >>> > >>> Let's not commit something that we are pretty sure is wrong. > >> > >> Jonathan, don't you just want to make UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS be true on watchOS and tvOS? > > > > err, HAVE() > > Maybe? Shouldn't the definition of modifierFlags also be guarded by > HAVE(UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS), in that case? > > I'm a bit confused by the original content of this line in the first place. > It seems like it would only compile if the recognizer had a _modifierFlags > member variable, which it doesn't seem to have for watchOS. I would expect > this code to read: > > #if HAVE(UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS) > return recognizer.modifierFlags; > #else > return 0; > #endif 1) That's a method, not a member variable. 2) This is all because _modifierFlags used to exist on iOS as SPI and then became API (without the underscore) in iOS 13.4, it's got nothing to do with watchOS/etc., but your substitution is definitely wrong. 3) The header suggests that the watchOS 6 SDK has both the SPI and API versions available. So I think my suggestion is probably the right path forward, but you have alternative paths available if necessary.
Created attachment 401642 [details] Patch
Created attachment 401643 [details] Patch
Comment on attachment 401643 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401643&action=review > Source/WTF/wtf/PlatformHave.h:450 > +#if (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130400) As previously mentioned, a IOS_FAMILY + IPHONE_OS_VERSION* check is always wrong. > Source/WebKit/Platform/spi/ios/UIKitSPI.h:422 > -#if PLATFORM(IOS) && !defined(__IPHONE_13_4) > +#if !HAVE(UI_GESTURE_RECOGNIZER_MODIFIER_FLAGS) This used defined(__IPHONE_13_4) instead of MIN_REQUIRED for a reason! Blame it and see.
Created attachment 401668 [details] Patch
(In reply to Jonathan Bedard from comment #13) > Created attachment 401668 [details] > Patch I think I was trying way too hard. Seems that we don't need the SPI header at all. And actually, do we still need the SPI declaration for iOS once we move to 13.5, the old macros only work on 13.4, right?
Comment on attachment 401668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401668&action=review > Source/WTF/wtf/PlatformHave.h:450 > +#if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130400 || PLATFORM(WATCHOS) || PLATFORM(APPLETV)) Seems like we could either remove the () entirely, or put them just around the && section.
Created attachment 401677 [details] Patch for landing
Committed r262924: <https://trac.webkit.org/changeset/262924> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401677 [details].