REGRESSION(r254389): Cordova throws an exception because it expects a hyphen inside navigator.locale
Created attachment 393329 [details] Patch
<rdar://problem/59845517>
Comment on attachment 393329 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393329&action=review > Source/WTF/wtf/cocoa/LanguageCocoa.mm:40 > +#elif PLATFORM(IOS_FAMILY) > + static bool applicationVersionCheck = dyld_get_program_sdk_version() >= DYLD_IOS_VERSION_13_4; I want to make sure this platform and version checking is correct. The PLATFORM(IOS_FAMILY) check will be True for tvOS and watchOS, as well as iOS. But that DYLD_IOS_VERSION_13_4 seems to be iOS-only. So should the pre-processor conditional actually be just PLATFORM(IOS)? And what should happen with macCatalyst? PLATFORM(IOS) will return False for macCatalyst, so if we want to support that platform, we'd also need a PLATFORM(MACCATALYST) check (I assume the iOS version number would be appropriate for that platform?).
Comment on attachment 393329 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393329&action=review > Source/WTF/wtf/cocoa/LanguageCocoa.mm:38 > + static bool applicationVersionCheck = dyld_get_program_sdk_version() >= DYLD_MACOSX_VERSION_10_15_4; I don't think calling dyld_get_program_sdk_version() in the WebKit2 WebContent process will do what you want. AFAIK, dyld_get_program_sdk_version() will only do what you want in the wK2 UIProcess or WK1.
Comment on attachment 393329 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393329&action=review >> Source/WTF/wtf/cocoa/LanguageCocoa.mm:38 >> + static bool applicationVersionCheck = dyld_get_program_sdk_version() >= DYLD_MACOSX_VERSION_10_15_4; > > I don't think calling dyld_get_program_sdk_version() in the WebKit2 WebContent process will do what you want. AFAIK, dyld_get_program_sdk_version() will only do what you want in the wK2 UIProcess or WK1. Good catch! I'm pursuing this in https://bugs.webkit.org/show_bug.cgi?id=209030.
Comment on attachment 393329 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393329&action=review >> Source/WTF/wtf/cocoa/LanguageCocoa.mm:40 >> + static bool applicationVersionCheck = dyld_get_program_sdk_version() >= DYLD_IOS_VERSION_13_4; > > I want to make sure this platform and version checking is correct. The PLATFORM(IOS_FAMILY) check will be True for tvOS and watchOS, as well as iOS. But that DYLD_IOS_VERSION_13_4 seems to be iOS-only. So should the pre-processor conditional actually be just PLATFORM(IOS)? And what should happen with macCatalyst? PLATFORM(IOS) will return False for macCatalyst, so if we want to support that platform, we'd also need a PLATFORM(MACCATALYST) check (I assume the iOS version number would be appropriate for that platform?). From my testing: tvOS: 0xD0400 watchOS: 0xD0200 macOS: 0xA0F04 iOS: 0xD0400
Created attachment 393557 [details] WIP
Created attachment 393565 [details] Patch
Comment on attachment 393565 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393565&action=review > Source/WTF/wtf/cocoa/LanguageCocoa.mm:44 > +#if PLATFORM(MAC) > + static bool applicationVersionCheck = applicationSDKVersion() >= DYLD_MACOSX_VERSION_10_15_4; > +#elif PLATFORM(IOS) > + static bool applicationVersionCheck = applicationSDKVersion() >= DYLD_IOS_VERSION_13_4; > +#elif > + static bool applicationVersionCheck = true; > +#endif > + static bool result = applicationVersionCheck && [NSLocale respondsToSelector:@selector(minimizedLanguagesFromLanguages:)]; I suggest static const bool or static constexpr for all of these. But also, not elegant to have two globals since the first is only used to compute the second. So write it like this: static const bool result = [] { #if PLATFORM(MAC) if (applicationSDKVersion() < DYLD_MACOSX_VERSION_10_15_4) return false; #endif #if PLATFORM(IOS) if (applicationSDKVersion() < DYLD_IOS_VERSION_13_4) return false; #elif return [NSLocale respondsToSelector:@selector(minimizedLanguagesFromLanguages:)]; }();
Created attachment 393810 [details] Patch for committing
Committed r258658: <https://trac.webkit.org/changeset/258658> All reviewed patches have been landed. Closing bug and clearing flags on attachment 393810 [details].