HAVE_ACCESSIBILITY_BUNDLES_PATH is defined like so: >#if PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000 >#define HAVE_ACCESSIBILITY_BUNDLES_PATH 1 >#endif The use of PLATFORM(IOS_FAMILY) suggests we should enable this HAVE() on some tvOS and watchOS platforms, but __IPHONE_OS_VERSION_MIN_REQUIRED will never be >= 140000 on these platforms. If we only intend to support iOS, we should change PLATFORM(IOS_FAMILY) to PLATFORM(IOS). Otherwise, we need to add a version check for each platform we intend to support, e.g.: >(PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) \ > || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 60000) \ > || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 130000)
<rdar://problem/63931340>
Created attachment 401380 [details] patch
Comment on attachment 401380 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=401380&action=review > Source/WTF/wtf/PlatformHave.h:388 > -#if PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000 > +#if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) \ > + || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 70000) \ > + || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 140000) Seems like an improvement. Also note that in IOS_FAMILY, besides iOS, watchOS, and tvOS, there is Mac Catalyst.
Comment on attachment 401380 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=401380&action=review >> Source/WTF/wtf/PlatformHave.h:388 >> + || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 140000) > > Seems like an improvement. Also note that in IOS_FAMILY, besides iOS, watchOS, and tvOS, there is Mac Catalyst. yes good point. we should also add that
Created attachment 401388 [details] patch
Comment on attachment 401388 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=401388&action=review > Source/WTF/wtf/PlatformHave.h:389 > + || (PLATFORM(MACCATALYST) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) Tim Horton asked earlier if __IPHONE_OS_VERSION_MIN_REQUIRED was the correct macro to check for MACCATALYST. He was pretty sure it was not.
(In reply to Darin Adler from comment #6) > Comment on attachment 401388 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=401388&action=review > > > Source/WTF/wtf/PlatformHave.h:389 > > + || (PLATFORM(MACCATALYST) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) > > Tim Horton asked earlier if __IPHONE_OS_VERSION_MIN_REQUIRED was the correct > macro to check for MACCATALYST. He was pretty sure it was not. I think it works I don't know if it's the best one. I saw this was already being used #if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101600) || (PLATFORM(MACCATALYST) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 140000) #define HAVE_MEDIA_USAGE_FRAMEWORK 1 #endif
OK. maybe HAVE(MEDIA_USAGE_FRAMEWORK) is right and Tim is wrong. Or maybe ... the other way around? Sure would be nice to confirm.
(In reply to Darin Adler from comment #8) > OK. maybe HAVE(MEDIA_USAGE_FRAMEWORK) is right and Tim is wrong. Or maybe > ... the other way around? Sure would be nice to confirm. It looks like I was wrong! (or am wrong now, anyway!)
Committed r262765: <https://trac.webkit.org/changeset/262765> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401388 [details].