From 85c39faa9b30e71f5b263ccc4009318cee985c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=2E=20P=2E=20Krasi=C5=84ski-Sroka?= Date: Thu, 1 Aug 2024 20:51:06 +0200 Subject: [PATCH] Properly calculate alwaysUse24HourFormat on MacOS (flutter/engine#53795) Moves the implementation if isAlwaysUse24HourFormat from iOS's FlutterViewController internals to common utility, and makes use of it on MacOS in order to return correct value of `alwaysUse24HourFormat`. This PR partially resolves [#32006](https://github.com/flutter/flutter/issues/32006). Note that on iOS 16+ and MacOS 13+, there is a new API for obtaining this information: https://developer.apple.com/documentation/foundation/locale/components/3952289-hourcycle. However, to keep things simpler, I wanted to not include changes to the logic. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../ci/licenses_golden/licenses_flutter | 4 ++ .../shell/platform/darwin/common/BUILD.gn | 1 + .../framework/Headers/FlutterHourFormat.h | 15 ++++++++ .../framework/Source/FlutterHourFormat.mm | 26 +++++++++++++ .../darwin/common/framework_common.gni | 1 + .../framework/Headers/FlutterViewController.h | 1 + .../framework/Source/FlutterViewController.mm | 20 +--------- .../Source/FlutterViewControllerTest.mm | 30 +++++++++++++++ .../macos/framework/Headers/FlutterEngine.h | 1 + .../macos/framework/Source/FlutterEngine.mm | 2 +- .../framework/Source/FlutterEngineTest.mm | 38 +++++++++++++++++++ 11 files changed, 119 insertions(+), 20 deletions(-) create mode 100644 engine/src/flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h create mode 100644 engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterHourFormat.mm diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index bbd92f1c6e..f73dac77ae 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -43580,6 +43580,7 @@ ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterB ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterCodecs.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterDartProject.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterTexture.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h + ../../../flutter/LICENSE @@ -43588,6 +43589,7 @@ ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterBi ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterChannels.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterHourFormat.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterNSBundleUtils.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterNSBundleUtils.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodec.mm + ../../../flutter/LICENSE @@ -46480,6 +46482,7 @@ FILE: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterBin FILE: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h FILE: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterCodecs.h FILE: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterDartProject.h +FILE: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h FILE: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h FILE: ../../../flutter/shell/platform/darwin/common/framework/Headers/FlutterTexture.h FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h @@ -46488,6 +46491,7 @@ FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterBina FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterChannels.mm FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm +FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterHourFormat.mm FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterNSBundleUtils.h FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterNSBundleUtils.mm FILE: ../../../flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodec.mm diff --git a/engine/src/flutter/shell/platform/darwin/common/BUILD.gn b/engine/src/flutter/shell/platform/darwin/common/BUILD.gn index f86af90171..3d509700e0 100644 --- a/engine/src/flutter/shell/platform/darwin/common/BUILD.gn +++ b/engine/src/flutter/shell/platform/darwin/common/BUILD.gn @@ -85,6 +85,7 @@ source_set("framework_common") { "framework/Source/FlutterBinaryMessengerRelay.mm", "framework/Source/FlutterChannels.mm", "framework/Source/FlutterCodecs.mm", + "framework/Source/FlutterHourFormat.mm", "framework/Source/FlutterNSBundleUtils.h", "framework/Source/FlutterNSBundleUtils.mm", "framework/Source/FlutterStandardCodec.mm", diff --git a/engine/src/flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h b/engine/src/flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h new file mode 100644 index 0000000000..e33e1a0ac6 --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h @@ -0,0 +1,15 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_COMMON_FRAMEWORK_HEADERS_FLUTTERHOURFORMAT_H_ +#define FLUTTER_SHELL_PLATFORM_DARWIN_COMMON_FRAMEWORK_HEADERS_FLUTTERHOURFORMAT_H_ + +#import + +@interface FlutterHourFormat : NSObject ++ (BOOL)isAlwaysUse24HourFormat; + +@end + +#endif // FLUTTER_SHELL_PLATFORM_DARWIN_COMMON_FRAMEWORK_HEADERS_FLUTTERHOURFORMAT_H_ diff --git a/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterHourFormat.mm b/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterHourFormat.mm new file mode 100644 index 0000000000..a3b80cf073 --- /dev/null +++ b/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterHourFormat.mm @@ -0,0 +1,26 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h" + +@implementation FlutterHourFormat ++ (BOOL)isAlwaysUse24HourFormat { + // iOS does not report its "24-Hour Time" user setting in the API. Instead, it applies + // it automatically to NSDateFormatter when used with [NSLocale currentLocale]. It is + // essential that [NSLocale currentLocale] is used. Any custom locale, even the one + // that's the same as [NSLocale currentLocale] will ignore the 24-hour option (there + // must be some internal field that's not exposed to developers). + // + // Therefore this option behaves differently across Android and iOS. On Android this + // setting is exposed standalone, and can therefore be applied to all locales, whether + // the "current system locale" or a custom one. On iOS it only applies to the current + // system locale. Widget implementors must take this into account in order to provide + // platform-idiomatic behavior in their widgets. + NSString* dateFormat = [NSDateFormatter dateFormatFromTemplate:@"j" + options:0 + locale:[NSLocale currentLocale]]; + return [dateFormat rangeOfString:@"a"].location == NSNotFound; +} + +@end diff --git a/engine/src/flutter/shell/platform/darwin/common/framework_common.gni b/engine/src/flutter/shell/platform/darwin/common/framework_common.gni index 02bb8b7dfb..e2fa8eedad 100644 --- a/engine/src/flutter/shell/platform/darwin/common/framework_common.gni +++ b/engine/src/flutter/shell/platform/darwin/common/framework_common.gni @@ -8,6 +8,7 @@ framework_common_headers = "framework/Headers/FlutterBinaryMessenger.h", "framework/Headers/FlutterChannels.h", "framework/Headers/FlutterCodecs.h", + "framework/Headers/FlutterHourFormat.h", "framework/Headers/FlutterTexture.h", "framework/Headers/FlutterDartProject.h", ], diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h index a218fd3782..7e2ea40fb8 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h @@ -11,6 +11,7 @@ #import "FlutterBinaryMessenger.h" #import "FlutterDartProject.h" #import "FlutterEngine.h" +#import "FlutterHourFormat.h" #import "FlutterMacros.h" #import "FlutterPlugin.h" #import "FlutterTexture.h" diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 096c5a244e..60bff2c482 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -2158,7 +2158,7 @@ static flutter::PointerData::DeviceKind DeviceKindFromTouchType(UITouch* touch) - (void)onUserSettingsChanged:(NSNotification*)notification { [[_engine.get() settingsChannel] sendMessage:@{ @"textScaleFactor" : @([self textScaleFactor]), - @"alwaysUse24HourFormat" : @([self isAlwaysUse24HourFormat]), + @"alwaysUse24HourFormat" : @([FlutterHourFormat isAlwaysUse24HourFormat]), @"platformBrightness" : [self brightnessMode], @"platformContrast" : [self contrastMode], @"nativeSpellCheckServiceDefined" : @true, @@ -2232,24 +2232,6 @@ static flutter::PointerData::DeviceKind DeviceKindFromTouchType(UITouch* touch) } } -- (BOOL)isAlwaysUse24HourFormat { - // iOS does not report its "24-Hour Time" user setting in the API. Instead, it applies - // it automatically to NSDateFormatter when used with [NSLocale currentLocale]. It is - // essential that [NSLocale currentLocale] is used. Any custom locale, even the one - // that's the same as [NSLocale currentLocale] will ignore the 24-hour option (there - // must be some internal field that's not exposed to developers). - // - // Therefore this option behaves differently across Android and iOS. On Android this - // setting is exposed standalone, and can therefore be applied to all locales, whether - // the "current system locale" or a custom one. On iOS it only applies to the current - // system locale. Widget implementors must take this into account in order to provide - // platform-idiomatic behavior in their widgets. - NSString* dateFormat = [NSDateFormatter dateFormatFromTemplate:@"j" - options:0 - locale:[NSLocale currentLocale]]; - return [dateFormat rangeOfString:@"a"].location == NSNotFound; -} - // The brightness mode of the platform, e.g., light or dark, expressed as a string that // is understood by the Flutter framework. See the settings // system channel for more information. diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm index b87260c290..7237967074 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm @@ -10,6 +10,7 @@ #include "flutter/lib/ui/window/pointer_data.h" #import "flutter/lib/ui/window/viewport_metrics.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h" +#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterHourFormat.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" #import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterEmbedderKeyResponder.h" @@ -1335,6 +1336,35 @@ extern NSNotificationName const FlutterViewControllerWillDealloc; [mockTraitCollection stopMocking]; } +- (void)testItReportsAlwaysUsed24HourFormat { + // Setup test. + id settingsChannel = OCMStrictClassMock([FlutterBasicMessageChannel class]); + OCMStub([self.mockEngine settingsChannel]).andReturn(settingsChannel); + FlutterViewController* vc = [[FlutterViewController alloc] initWithEngine:self.mockEngine + nibName:nil + bundle:nil]; + // Test the YES case. + id mockHourFormat = OCMClassMock([FlutterHourFormat class]); + OCMStub([mockHourFormat isAlwaysUse24HourFormat]).andReturn(YES); + OCMExpect([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) { + return [message[@"alwaysUse24HourFormat"] isEqual:@(YES)]; + }]]); + [vc onUserSettingsChanged:nil]; + [mockHourFormat stopMocking]; + + // Test the NO case. + mockHourFormat = OCMClassMock([FlutterHourFormat class]); + OCMStub([mockHourFormat isAlwaysUse24HourFormat]).andReturn(NO); + OCMExpect([settingsChannel sendMessage:[OCMArg checkWithBlock:^BOOL(id message) { + return [message[@"alwaysUse24HourFormat"] isEqual:@(NO)]; + }]]); + [vc onUserSettingsChanged:nil]; + [mockHourFormat stopMocking]; + + // Clean up mocks. + [settingsChannel stopMocking]; +} + - (void)testItReportsAccessibilityOnOffSwitchLabelsFlagNotSet { if (@available(iOS 13, *)) { // noop diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h index cebdf9005e..306bfceb08 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h @@ -12,6 +12,7 @@ #import "FlutterAppLifecycleDelegate.h" #import "FlutterBinaryMessenger.h" #import "FlutterDartProject.h" +#import "FlutterHourFormat.h" #import "FlutterMacros.h" #import "FlutterPluginRegistrarMacOS.h" #import "FlutterTexture.h" diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 9f00886607..2c79ee079d 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -964,7 +964,7 @@ static void SetThreadPriority(FlutterThreadPriority priority) { @"platformBrightness" : [brightness isEqualToString:@"Dark"] ? @"dark" : @"light", // TODO(jonahwilliams): https://github.com/flutter/flutter/issues/32006. @"textScaleFactor" : @1.0, - @"alwaysUse24HourFormat" : @false + @"alwaysUse24HourFormat" : @([FlutterHourFormat isAlwaysUse24HourFormat]), }]; } diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index d2858837c7..b7d8eb2311 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -1268,6 +1268,44 @@ TEST_F(FlutterEngineTest, DisplaySizeIsInPhysicalPixel) { engine = nil; } +TEST_F(FlutterEngineTest, ReportsHourFormat) { + __block BOOL expectedValue; + + // Set up mocks. + id channelMock = OCMClassMock([FlutterBasicMessageChannel class]); + OCMStub([channelMock messageChannelWithName:@"flutter/settings" + binaryMessenger:[OCMArg any] + codec:[OCMArg any]]) + .andReturn(channelMock); + OCMStub([channelMock sendMessage:[OCMArg any]]).andDo((^(NSInvocation* invocation) { + __weak id message; + [invocation getArgument:&message atIndex:2]; + EXPECT_EQ(message[@"alwaysUse24HourFormat"], @(expectedValue)); + })); + + id mockHourFormat = OCMClassMock([FlutterHourFormat class]); + OCMStub([mockHourFormat isAlwaysUse24HourFormat]).andDo((^(NSInvocation* invocation) { + [invocation setReturnValue:&expectedValue]; + })); + + id engineMock = CreateMockFlutterEngine(nil); + + // Verify the YES case. + expectedValue = YES; + EXPECT_TRUE([engineMock runWithEntrypoint:@"main"]); + [engineMock shutDownEngine]; + + // Verify the NO case. + expectedValue = NO; + EXPECT_TRUE([engineMock runWithEntrypoint:@"main"]); + [engineMock shutDownEngine]; + + // Clean up mocks. + [mockHourFormat stopMocking]; + [engineMock stopMocking]; + [channelMock stopMocking]; +} + } // namespace flutter::testing // NOLINTEND(clang-analyzer-core.StackAddressEscape)