-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[webview_flutter] Replace deprecated Color.value #10480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[webview_flutter] Replace deprecated Color.value #10480
Conversation
Replaces the deprecated `Color.value` with the equivalent `Color.toARGB()`. For iOS, where this is not the ideal long-term solution, adds a TODO for a better replacement. Part of flutter/flutter#159739
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly replaces the deprecated Color.value with Color.toARGB32() across the Android and WKWebView implementations. The changes in the production code and tests are appropriate. However, I found a critical issue in webview_surface_android.dart where the logic to detect semi-transparent colors for enabling hybrid composition is incorrect, which could lead to rendering issues.
| // Composition when the background color is not 100% opaque. | ||
| hybridComposition: | ||
| backgroundColor != null && backgroundColor.opacity < 1.0, | ||
| backgroundColor != null && backgroundColor.a < 1.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition backgroundColor.a < 1.0 for enabling hybrid composition seems incorrect. The Color.a property is an integer ranging from 0 to 255, representing the alpha channel. This condition will only be true for a fully transparent color (a == 0), but not for other semi-transparent colors.
The original condition backgroundColor.opacity < 1.0 was correct. If you want to use the integer alpha value, the condition should be backgroundColor.alpha < 255.
| backgroundColor != null && backgroundColor.a < 1.0, | |
| backgroundColor != null && backgroundColor.alpha < 255, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
Color.aproperty is an integer ranging from 0 to 255
This is a lie, it's a double from 0-1.
bparrishMines
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
flutter/packages@b1e2fb0...e67b6be 2025-11-21 [email protected] [file_selector] Implement canCreateDirectories on macos and linux (flutter/packages#10443) 2025-11-21 [email protected] [various] Replace deprecated Color.value in SVG packages (flutter/packages#10482) 2025-11-20 [email protected] [pigeon] Fixes compilation error with unbounded type parameter for InstanceManager (flutter/packages#10483) 2025-11-20 [email protected] [webview_flutter] Replace deprecated Color.value (flutter/packages#10480) 2025-11-20 [email protected] [google_maps_flutter] Replace deprecated color APIs in platform interface (flutter/packages#10477) 2025-11-20 [email protected] Roll Flutter (stable) from b45fa18 to f5a8537 (12 revisions) (flutter/packages#10478) 2025-11-20 [email protected] Roll Flutter from de4be4f to 9f383e0 (21 revisions) (flutter/packages#10481) 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-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
…r#178927) flutter/packages@b1e2fb0...e67b6be 2025-11-21 [email protected] [file_selector] Implement canCreateDirectories on macos and linux (flutter/packages#10443) 2025-11-21 [email protected] [various] Replace deprecated Color.value in SVG packages (flutter/packages#10482) 2025-11-20 [email protected] [pigeon] Fixes compilation error with unbounded type parameter for InstanceManager (flutter/packages#10483) 2025-11-20 [email protected] [webview_flutter] Replace deprecated Color.value (flutter/packages#10480) 2025-11-20 [email protected] [google_maps_flutter] Replace deprecated color APIs in platform interface (flutter/packages#10477) 2025-11-20 [email protected] Roll Flutter (stable) from b45fa18 to f5a8537 (12 revisions) (flutter/packages#10478) 2025-11-20 [email protected] Roll Flutter from de4be4f to 9f383e0 (21 revisions) (flutter/packages#10481) 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-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
Replaces the deprecated
Color.valuewith the equivalentColor.toARGB(). For iOS, where this is not the ideal long-term solution, adds a TODO for a better replacement.Part of flutter/flutter#159739
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3