[webview_flutter_wkwebview] Fixes lossless transition from Dart Color to native UIColor when calling setBackgroundColor#10610
Conversation
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the webview_flutter_wkwebview package to use UIColor for setting background colors instead of an integer representation, which is a good improvement for type safety and correctness. However, I've found a critical issue in the Dart code where Color properties are accessed incorrectly (.r, .g, etc. instead of .red, .green, etc.) and not normalized for the UIColor constructor. This will cause compilation errors. I've provided suggestions to fix this in the implementation and the tests. Additionally, a test case for setBackgroundColor was commented out, which I've suggested to re-enable with the necessary fixes.
packages/webview_flutter/webview_flutter_wkwebview/lib/src/legacy/web_kit_webview_widget.dart
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/lib/src/webkit_webview_controller.dart
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/test/legacy/web_kit_webview_widget_test.dart
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/test/webkit_webview_controller_test.dart
Outdated
Show resolved
Hide resolved
|
From triage: still something we plan to get back to, just not currently active. |
bparrishMines
requested review from
LongCatIsLooong and
hellohuanlin
as code owners
bparrishMines
changed the title
setBackgroundColor
There was a problem hiding this comment.
Code Review
This pull request aims to fix a lossy color transition by using a UIColor proxy instead of an integer representation. While the overall approach is correct, there's a critical issue in the implementation. The color components from Dart's Color object (which are integers from 0-255) are being passed directly to the UIColor constructor, which expects double values normalized to the range [0.0, 1.0]. This will result in incorrect colors being displayed. I've added suggestions to normalize these values by dividing by 255.0. The corresponding tests also need to be updated to check for these normalized values.
packages/webview_flutter/webview_flutter_wkwebview/lib/src/legacy/web_kit_webview_widget.dart
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/lib/src/webkit_webview_controller.dart
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/test/legacy/web_kit_webview_widget_test.dart
Show resolved
Hide resolved
packages/webview_flutter/webview_flutter_wkwebview/test/webkit_webview_controller_test.dart
Show resolved
Hide resolved
| let red = 0.5 | ||
| let green = 0.5 | ||
| let blue = 0.5 | ||
| let alpha = 0.5 |
There was a problem hiding this comment.
We should use different values for each component; this test would pass even if we had a typo somewhere that swapped colors around.
| @@ -1,3 +1,7 @@ | |||
| ## 3.23.8 | |||
|
|
|||
| * Fixes lossless transition from Dart Color to native UIColor when calling `setBackgroundColor`. | |||
There was a problem hiding this comment.
Nit: what it fixes is a potentially lossy transition; the fix is that it's now lossless. So it should be "Fixes lossy transition ..." or "Uses lossless conversion ..."
bparrishMines
added
the
autosubmit
…Dart Color to native UIColor when calling `setBackgroundColor` (flutter/packages#10610)
flutter/packages@a27d7c5...faa4e22 2026-03-02 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump the test-dependencies group across 14 directories with 1 update (flutter/packages#11152) 2026-02-28 [email protected] [pigeon] Reduce reply-handling duplication in generated Dart code. (flutter/packages#11114) 2026-02-27 [email protected] [webview_flutter_wkwebview] Fixes lossless transition from Dart Color to native UIColor when calling `setBackgroundColor` (flutter/packages#10610) 2026-02-27 [email protected] Roll Flutter from b31548f to 1141b2b (56 revisions) (flutter/packages#11137) 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#183117) flutter/packages@a27d7c5...faa4e22 2026-03-02 49699333+dependabot[bot]@users.noreply.github.com [dependabot]: Bump the test-dependencies group across 14 directories with 1 update (flutter/packages#11152) 2026-02-28 [email protected] [pigeon] Reduce reply-handling duplication in generated Dart code. (flutter/packages#11114) 2026-02-27 [email protected] [webview_flutter_wkwebview] Fixes lossless transition from Dart Color to native UIColor when calling `setBackgroundColor` (flutter/packages#10610) 2026-02-27 [email protected] Roll Flutter from b31548f to 1141b2b (56 revisions) (flutter/packages#11137) 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
2 participants
Fixes flutter/flutter#178870
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.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
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.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