Repository navigation
macOS: Compare accessibility strings as NSStrings in tests - #191853
Merged
cbracken merged 1 commit intoAug 28, 2026
Merged
Conversation
We were converting `NSString` instances to `std::string`s purely to compare them against C string literals. Comparing the `NSString` directly is shorter, matches the neighbouring role assertions, and drops five `UTF8String` conversions. Round-tripping these through `std::string` results in any embedded NULs triggering truncation which could, in theory, hide bugs. Spotted while working on: flutter#191616
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors unit tests in FlutterEngineTest.mm and FlutterPlatformNodeDelegateMacTest.mm by replacing C++ std::string conversions and comparisons of Objective-C string properties with direct isEqualToString: comparisons. There are no review comments, and I have no feedback to provide.
This was referenced Aug 27, 2026
LongCatIsLooong
approved these changes
Aug 28, 2026
pull Bot
pushed a commit
to Mu-L/flutter
that referenced
this pull request
Aug 28, 2026
…1855) `FlutterPlatformViewsController` was holding its view factories and their gesture blocking policies in `std::unordered_map`s keyed by `std::string`, so both the registration path and the create path converted an `NSString` to a `std::string` first. But all the mapped values are Obj-C types so we can migrate to na `NSMutableDictionary`. This matches how we do it in the macOS embedder. This removes the last of the `UTF8String` conversions in this file. As noted in flutter#191853 and flutter#191854 `std::string`'s `const char*` constructor determines the length using `strlen`, so it truncates at an embedded NUL and dereferences null if `UTF8String` ever returns NULL, which it does for a nil receiver. Spotted while working on: flutter#191616 No new tests since this just changes the implementation without changing any semantics. <!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
pull Bot
pushed a commit
to TheRakeshPurohit/flutter
that referenced
this pull request
Aug 28, 2026
…er#191854) `AccessibilityBridgeMacSpy` was recording the notifications it saw in a `std::unordered_map<std::string, gfx::NativeViewAccessible>`, converting the `NSAccessibilityNotificationName` key to a `std::string` on the way in and back again on the way out. Both the key and value are Obj-C objects, so we can use an `NSMutableDictionary` and avoid all the conversions. Similar to flutter#191853, `std::string` conversions use `strlen` to determine the length, which truncates at any embedded NUL chars, potentially hiding bugs. Spotted while working on: flutter#191616 <!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This was referenced Aug 28, 2026
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Aug 28, 2026
flutter/flutter@e8dca90...8d8c1aa 2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965) 2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931) 2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329) 2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960) 2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930) 2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953) 2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942) 2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934) 2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861) 2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912) 2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743) 2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929) 2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773) 2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853) 2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854) 2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927) 2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923) 2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930) 2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595) 2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911) 2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850) 2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906) 2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855) 2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895) 2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592) 2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913) 2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…r#12679) flutter/flutter@e8dca90...8d8c1aa 2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965) 2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931) 2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329) 2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960) 2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930) 2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953) 2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942) 2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934) 2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861) 2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912) 2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743) 2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929) 2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773) 2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853) 2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854) 2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927) 2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923) 2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930) 2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595) 2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911) 2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850) 2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906) 2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855) 2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895) 2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592) 2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913) 2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni
pushed a commit
to victorsanni/packages
that referenced
this pull request
Sep 9, 2026
…r#12679) flutter/flutter@e8dca90...8d8c1aa 2026-08-28 [email protected] Roll Packages from bd3cbc1 to cd4cdd0 (15 revisions) (flutter/flutter#191965) 2026-08-28 [email protected] Bump customer testing version to pick up forui tests (flutter/flutter#191931) 2026-08-28 [email protected] Use leading scroll offset instead of infinite offset to reveal pinned slivers (flutter/flutter#191329) 2026-08-28 [email protected] Roll Skia from 21f71051e749 to fe0b9a5261f9 (1 revision) (flutter/flutter#191960) 2026-08-28 [email protected] Share the common Linux window controller code in mixins (flutter/flutter#191930) 2026-08-28 [email protected] Roll Dart SDK from 2cf2f69fd9f0 to ae63a6eca3f1 (1 revision) (flutter/flutter#191953) 2026-08-28 [email protected] Roll Skia from 11e3a7cb0fd0 to 21f71051e749 (3 revisions) (flutter/flutter#191942) 2026-08-28 [email protected] Roll Dart SDK from e030e77d5413 to 2cf2f69fd9f0 (2 revisions) (flutter/flutter#191934) 2026-08-28 [email protected] Remove no-shuffle in flutter tools overall experience test (flutter/flutter#191861) 2026-08-28 [email protected] [flutter_tools] Gracefully handle Windows Application Control and security policy execution blocks (flutter/flutter#191912) 2026-08-28 [email protected] [tool] Migrate ConfigCommand to modular dependency injection (flutter/flutter#190743) 2026-08-28 [email protected] Roll Skia from 37459da0a755 to 11e3a7cb0fd0 (9 revisions) (flutter/flutter#191929) 2026-08-28 [email protected] Connect to moved-to-rect after the window is realized (flutter/flutter#191773) 2026-08-28 [email protected] macOS: Compare accessibility strings as NSStrings in tests (flutter/flutter#191853) 2026-08-28 [email protected] macOS: Record test accessibility notifications in a dictionary (flutter/flutter#191854) 2026-08-28 [email protected] Roll Fuchsia Test Scripts from 6SXRz6wsA2Zm_rAqP... to u_fSMsPU22VvUsvSo... (flutter/flutter#191927) 2026-08-28 [email protected] Roll Dart SDK from cb4c39d067f3 to e030e77d5413 (3 revisions) (flutter/flutter#191923) 2026-08-27 [email protected] Migrate ios driver tests batch 2 to simulators (flutter/flutter#190930) 2026-08-27 [email protected] [flutter_test] Ensure LiveTestWidgetsFlutterBinding executes frame and pointer callbacks in the test zone (flutter/flutter#191595) 2026-08-27 [email protected] Deprecate legacy gen_defaults tool (flutter/flutter#191911) 2026-08-27 [email protected] Fix black window in the subsurface view renderer (flutter/flutter#191850) 2026-08-27 [email protected] Roll Skia from 82e972ea84b1 to 37459da0a755 (10 revisions) (flutter/flutter#191906) 2026-08-27 [email protected] iOS: Key the platform view factory registries by NSString (flutter/flutter#191855) 2026-08-27 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#191895) 2026-08-27 [email protected] [flutter_tools] Honor configured build-dir across flutter project and build targets (flutter/flutter#191592) 2026-08-27 [email protected] Revert: Handle linear and radial gradients in UberSDF (flutter/flutter#191913) 2026-08-27 [email protected] Explicitly disable HCPP in hybrid_android_views integration test (flutter/flutter#191894) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We were converting
NSStringinstances tostd::strings purely to compare them against C string literals. Comparing theNSStringdirectly is shorter, matches the neighbouring role assertions, and drops fiveUTF8Stringconversions.Round-tripping these through
std::stringresults in any embedded NULs triggering truncation which could, in theory, hide bugs.Spotted while working on: #191616
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.