Remove some support for iOS versions less than 13.0. Update conditionals that reference __IPHONE_OS_VERSION_MIN_REQUIRED and __IPHONE_OS_VERSION_MAX_ALLOWED, assuming that they both have values >= 130000. This means that expressions like "__IPHONE_OS_VERSION_MIN_REQUIRED < 101300" are always False and "__IPHONE_OS_VERSION_MIN_REQUIRED >= 101300" are always True. This removal is part of a series of patches effecting the removal of dead code for old versions of iOS. This particular pass involves changes in which Dean Jackson was involved. These changes are isolated from other similar changes in order to facilitate the reviewing process.
<rdar://problem/55853960>
Created attachment 379841 [details] Patch
Reviewers note: Please check that the conditional around HAVE_CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED is correct. It's not apparent to me if this should be enabled for all of the iOS family or just iOS per se.
Should these be enabled for macCatalyst? #define HAVE_CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED 1 #define USE_UICONTEXTMENU 1
Comment on attachment 379841 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379841&action=review > Source/WTF/wtf/Platform.h:1446 > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS) Seems ok to move to PLATFORM(IOS) but just want to double check we do not want PLATFORM(IOS_FAMILY).
Comment on attachment 379841 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379841&action=review >> Source/WTF/wtf/Platform.h:1446 >> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS) > > Seems ok to move to PLATFORM(IOS) but just want to double check we do not want PLATFORM(IOS_FAMILY). It does seem like this would apply to IOS_FAMILY. Or perhaps it should be "... PLATFORM(IOS) || PLATFORM(MACCATALYST)" ?
(In reply to Brent Fulgham from comment #6) > Comment on attachment 379841 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=379841&action=review > > >> Source/WTF/wtf/Platform.h:1446 > >> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS) > > > > Seems ok to move to PLATFORM(IOS) but just want to double check we do not want PLATFORM(IOS_FAMILY). > > It does seem like this would apply to IOS_FAMILY. Or perhaps it should be > "... PLATFORM(IOS) || PLATFORM(MACCATALYST)" ? It seems incredibly likely that this should be IOS_FAMILY.
> #define HAVE_CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED 1 This is the one at Platform.h:1446 that everyone commented on. I'll include this for IOS_FAMILY. > #define USE_UICONTEXTMENU 1 I'll include this for PLATFORM(IOS) || PLATFORM(MACCATALYST). It looks like the facilities it enables are defined on watch, but not tv, so this can't be for IOS_FAMILY. If someone thinks this should be enabled for watch, they can add that, but for now I'll keep the previous behavior.
Created attachment 380599 [details] Patch
Comment on attachment 380599 [details] Patch Clearing flags on attachment: 380599 Committed r250953: <https://trac.webkit.org/changeset/250953>
All reviewed patches have been landed. Closing bug.
Comment on attachment 380599 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380599&action=review > Source/WTF/wtf/Platform.h:1449 > -#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS_FAMILY) > #define HAVE_CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED 1 > #endif This seems like a change for watchOS, tvOS, and perhaps for Catalyst as well. Is it an intentional change that’s either valuable or harmless? I ask because it’s not mentioned in the change log, which only mentions removing support for older versions of iOS. And because if it’s valuable, there’s no test that validates the improvement. > Source/WTF/wtf/Platform.h:1620 > -#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000 > +#if PLATFORM(IOS) || PLATFORM(MACCATALYST) > #define USE_UICONTEXTMENU 1 > #endif This seems like a change in behavior for Catalyst. Is it an intentional change that’s valuable? I ask because there’s no mention in the change log and no additional testing to validate the improvement.
(In reply to Darin Adler from comment #12) > Comment on attachment 380599 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=380599&action=review > > > Source/WTF/wtf/Platform.h:1449 > > -#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) > > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS_FAMILY) > > #define HAVE_CA_WHERE_ADDITIVE_TRANSFORMS_ARE_REVERSED 1 > > #endif > > This seems like a change for watchOS, tvOS, and perhaps for Catalyst as > well. Is it an intentional change that’s either valuable or harmless? I ask > because it’s not mentioned in the change log, which only mentions removing > support for older versions of iOS. And because if it’s valuable, there’s no > test that validates the improvement. This is a valuable change. See https://trac.webkit.org/changeset/241852/webkit. The test from that commit can probably be made to work on watchOS/Catalyst, but test coverage in those contexts is... not ideal at the moment. > > Source/WTF/wtf/Platform.h:1620 > > -#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000 > > +#if PLATFORM(IOS) || PLATFORM(MACCATALYST) > > #define USE_UICONTEXTMENU 1 > > #endif > > This seems like a change in behavior for Catalyst. Is it an intentional > change that’s valuable? I ask because there’s no mention in the change log > and no additional testing to validate the improvement. This one surprises me more, but could very easily be right (and we have no Catalyst testing so that part is not a surprise).