[CP-stable][ Tool ] Fix flutter run -d all crash
#180867
Conversation
661b8ed introduced changes related to build hooks that made assumptions about the value of the detected target platform, effectively restricting `flutter run` to targeting single devices. This change fixes the regression which prevented developers from deploying their application to multiple devices with `flutter run -d all`. Fixes flutter#179857
|
This pull request was opened against a branch other than master. Since Flutter pull requests should not normally be opened against branches other than master, I have changed the base to master. If this was intended, you may modify the base back to stable. See the Release Process for information about how other branches get updated. Reviewers: Use caution before merging pull requests to branches other than master, unless this is an intentional hotfix/cherrypick. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request addresses a crash with flutter run -d all when multiple devices are connected. The fix correctly handles building assets for multiple target platforms. It also includes a related fix to prevent re-listening to streams during attachment, which could cause state errors.
Beyond the main fix, this PR bundles a large number of other changes, including:
- Fixes for Android Gradle Plugin 9+ compatibility.
- Improvements to the widget preview feature, such as more robust file watching and dependency management.
- A fix for web on-screen keyboard handling that prevents resizing the view.
- A fix for non-ASCII path handling on Windows.
- A workaround for a web view tappability issue on iOS 26.
- A fix for handling
NSAttributedStringin macOS text input. - Improvements to accessibility on Linux.
- Fixes for OpenGL state restoration in the Linux compositor.
- Various CI and dependency updates.
The changes are generally well-implemented and include corresponding tests. However, I found a potential issue in the Linux accessibility improvements that I've commented on. Given this is a cherry-pick to a stable branch, the large scope of unrelated changes is noteworthy.
I am having trouble creating individual review comments. Click here to see my feedback.
engine/src/flutter/shell/platform/linux/fl_accessible_node.cc (114-116)
The logic for is_sensitive appears to be incorrect. It returns true even when flags.is_enabled is kFlutterTristateFalse. A disabled widget should not be considered sensitive (i.e., able to be acted upon). This could lead to incorrect behavior in screen readers and other assistive technologies.
The implementation should likely check for kFlutterTristateTrue, similar to the is_enabled helper function.
static gboolean is_sensitive(FlutterSemanticsFlags flags) {
return flags.is_enabled == kFlutterTristateTrue;
}|
I think you have some syntax error in |
|
Are we cutting a new stable branch soon? |
That's what I get for fixing merge conflicts in vim 😓 Sorry about that, should be fixed now. |
walley892
left a comment
There was a problem hiding this comment.
LGTM. Will add this to the next stable hotfix 3.38.7.
e1fce2a
into
flutter:flutter-3.38-candidate.0
Updates the changelog to prepare for the 3.38.7 stable hotfix. Includes #180867
Updates the CHANGELOG following the 3.38.7 stable hotfix release. Includes Includes #180867
Updates the CHANGELOG following the 3.38.7 stable hotfix release. Includes Includes flutter#180867
Updates the CHANGELOG following the 3.38.7 stable hotfix release. Includes Includes flutter#180867
Issue Link:
What is the link to the issue this cherry-pick is addressing?
#179857
Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)?
Does it impact development (ex. flutter doctor crashes when Android Studio is installed),
or the shipping of production apps (the app crashes on launch).
This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick.
flutter run -d allcauses the tool to crash if multiple devices are available.Changelog Description:
Explain this cherry pick:
See best practices for examples.
flutter run -d allcauses the tool to crash if multiple devices are available.Workaround:
Is there a workaround for this issue?
No.
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
What are the steps to validate that this fix works?
Run
flutter run -d allwith multiple non-web devices available and verify the application is deployed to all non-web devices.