[vector_graphics_compiler] Fix rgb/rgba color parsing to support modern CSS syntax #10538

AlexDochioiu · 2025-11-30T11:55:19Z

Consolidate rgb() and rgba() parsing into single implementation
Add support for space-separated color values
Add support for percentage-based RGB values
Add support for slash (/) separator before alpha channel
Handle both comma and space delimiters
Add comprehensive test coverage for various color formats

Pre-Review Checklist

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-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.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

gemini-code-assist

Code Review

This pull request refactors the color parsing logic for rgb() and rgba() to support modern CSS syntax, including space-separated values, percentage-based values, and the slash separator for the alpha channel. The implementation is consolidated, and the old rgb parsing logic is removed. Comprehensive tests have been added to cover the new parsing capabilities.

The changes are well-structured and the added tests are thorough. I have one suggestion to improve the strictness of the parsing to better align with CSS standards.

gemini-code-assist · 2025-11-30T11:57:11Z

packages/vector_graphics_compiler/lib/src/svg/parser.dart

+          .map((indexedColor) {
+            var (index, rawColor) = indexedColor;
+            if (rawColor.endsWith('%')) {
+              rawColor = rawColor.substring(0, rawColor.length - 1);
+              return (parseDouble(rawColor)! * 2.55).round();
+            }
+            if (index == 3) {
+              // if alpha is not percentage, it means it's a double between 0 and 1
+              final double opacity = parseDouble(rawColor)!;
+              if (opacity < 0 || opacity > 1) {
+                throw StateError('Invalid "opacity": $opacity');
+              }
+              return (opacity * 255).round();
+            }
+            // If rgb is not percentage, it means it's an integer between 0 and 255
+            return int.parse(rawColor);
+          })


The use of the custom parseDouble from numbers.dart is not ideal for parsing color components. This function is designed to strip units like px or em, but according to the CSS specification, color components in rgb() or rgba() functions should not have units. This could lead to incorrect parsing of invalid color strings (e.g., rgb(50px, 0, 0) would be parsed as rgb(50, 0, 0)).

To ensure stricter adherence to the CSS standard, you should use double.parse from dart:core instead, which will correctly throw an error for values with units.

Suggested change

.map((indexedColor) {

var (index, rawColor) = indexedColor;

if (rawColor.endsWith('%')) {

rawColor = rawColor.substring(0, rawColor.length - 1);

return (parseDouble(rawColor)! * 2.55).round();

}

if (index == 3) {

// if alpha is not percentage, it means it's a double between 0 and 1

final double opacity = parseDouble(rawColor)!;

if (opacity < 0 || opacity > 1) {

throw StateError('Invalid "opacity": $opacity');

}

return (opacity * 255).round();

}

// If rgb is not percentage, it means it's an integer between 0 and 255

return int.parse(rawColor);

})

.map((indexedColor) {

var (index, rawColor) = indexedColor;

if (rawColor.endsWith('%')) {

rawColor = rawColor.substring(0, rawColor.length - 1);

return (double.parse(rawColor) * 2.55).round();

}

if (index == 3) {

// if alpha is not percentage, it means it's a double between 0 and 1

final double opacity = double.parse(rawColor);

if (opacity < 0 || opacity > 1) {

throw StateError('Invalid "opacity": $opacity');

}

return (opacity * 255).round();

}

// If rgb is not percentage, it means it's an integer between 0 and 255

return int.parse(rawColor);

})

This parseDouble usage was retained from the existing rbg parsing implementation.

AlexDochioiu · 2025-11-30T14:26:41Z

packages/vector_graphics_compiler/lib/src/svg/parser.dart

+      final List<int> rgba = colorString
          .substring(colorString.indexOf('(') + 1, colorString.indexOf(')'))
-          .split(',')
+          .split(RegExp(r'[,/\s]'))


valid delimiters are: ,, (empty space) and / (used to delimit the opacity)

This is not a correct reframing of the spec you linked to in the comment introducing this code. From what I see there, the only valid delimiters are either:

all spaces, except for alpha which uses /, or

all commas (legacy)

For example, rgb(255 / 255 255, 1.0) is not valid, but will be accepted by this code.

stuartmorgan-g · 2025-11-30T15:53:46Z

I linked to at least one issue that this PR fixes in the description above.

I updated pubspec.yaml with 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.

Thanks for the contribution! Once you've completed the checklist, please mark the PR as ready for review.

I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.

Please make sure to follow the linked style guide.

…rn CSS syntax - Consolidate rgb() and rgba() parsing into single implementation - Add support for space-separated color values - Add support for percentage-based RGB values - Add support for slash (/) separator before alpha channel - Handle both comma and space delimiters - Add comprehensive test coverage for various color formats

AlexDochioiu · 2025-12-01T08:12:26Z

@stuartmorgan-g Completed the checklist. This is my first PR to any flutter/dart repo. If anything is not exactly correct, please let me know and I'll address.

AlexDochioiu · 2025-12-16T08:19:51Z

Ping @stuartmorgan-g . Any chance we can get this fix reviewed/merged?

stuartmorgan-g · 2025-12-16T20:47:06Z

I can do an initial review, but this will need a review from the engine team. Review capacity tends to be low in December, which is probably why this hasn't been picked up yet.

stuartmorgan-g · 2025-12-16T20:48:13Z

packages/vector_graphics_compiler/lib/src/svg/parser.dart

+    // handle rgba() colors e.g. rgb(255, 255, 255) and rgba(255, 255, 255, 1.0)
+    // https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Values/color_value/rgb
+    if (colorString.toLowerCase().startsWith('rgba') ||
+        colorString.toLowerCase().startsWith('rgb')) {


rgba starts with rgb, so the first check here is duplicative.

stuartmorgan-g · 2025-12-16T20:52:55Z

packages/vector_graphics_compiler/lib/src/svg/parser.dart

+      final List<int> rgba = colorString
          .substring(colorString.indexOf('(') + 1, colorString.indexOf(')'))
-          .split(',')
+          .split(RegExp(r'[,/\s]'))


This is not a correct reframing of the spec you linked to in the comment introducing this code. From what I see there, the only valid delimiters are either:

all spaces, except for alpha which uses /, or

all commas (legacy)

For example, rgb(255 / 255 255, 1.0) is not valid, but will be accepted by this code.

stuartmorgan-g · 2025-12-16T20:57:47Z

packages/vector_graphics_compiler/lib/src/svg/parser.dart

-          .split(',')
+          .split(RegExp(r'[,/\s]'))
          .map((String rawColor) => rawColor.trim())
+          .where((e) => e.isNotEmpty)


This appears to be trying to correct for overly aggressive splitting (e.g., ", " as a delimiter), and will silently accept bad input in surprising ways.

For instance, this typo where someone using legacy syntax and forgot the green value will be accepted as an unexpected color instead of erroring out: rgba(255, , 255, 1.0)

stuartmorgan-g · 2025-12-16T21:02:28Z

packages/vector_graphics_compiler/test/parsers_test.dart

+        const Color.fromARGB(0xFF, 0xAB, 0xCD, 0xEF),
+      );
+    });
  });


The test suite should include various invalid variants, and ensure that they are handled as expected (I'm not familiar with the context of this code, but I'm assuming since the return type is nullable invalid values should likely return null).

stuartmorgan-g · 2025-12-16T21:02:55Z

packages/vector_graphics_compiler/CHANGELOG.md

-## NEXT
+## 1.1.20

+* Fix rgb and rgba color parsing to handle modern CSS syntax


This needs to follow the linked style guide; please see the notes about both verb form and punctuation.

AlexDochioiu requested a review from jtmcdole as a code owner November 30, 2025 11:55

github-actions bot added p: vector_graphics triage-engine Should be looked at in engine triage labels Nov 30, 2025

gemini-code-assist bot reviewed Nov 30, 2025

View reviewed changes

AlexDochioiu commented Nov 30, 2025

View reviewed changes

stuartmorgan-g marked this pull request as draft November 30, 2025 15:53

AlexDochioiu marked this pull request as ready for review December 1, 2025 07:53

AlexDochioiu marked this pull request as draft December 1, 2025 07:53

AlexDochioiu added 2 commits December 1, 2025 10:09

[vector_graphics_compiler] Bumped version and update CHANGELOG

16bf81a

AlexDochioiu force-pushed the fix/color-parsing branch from 057153b to 16bf81a Compare December 1, 2025 08:10

AlexDochioiu marked this pull request as ready for review December 1, 2025 08:10

AlexDochioiu mentioned this pull request Dec 2, 2025

[vector_graphics_compiler] Cannot handle modern css color syntax flutter/flutter#179261

Open

Merge branch 'main' into fix/color-parsing

ca102fa

stuartmorgan-g requested changes Dec 16, 2025

View reviewed changes

[vector_graphics_compiler] Fix rgb/rgba color parsing to support modern CSS syntax #10538

Are you sure you want to change the base?

[vector_graphics_compiler] Fix rgb/rgba color parsing to support modern CSS syntax #10538

Uh oh!

Conversation

AlexDochioiu commented Nov 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Nov 30, 2025

Uh oh!

AlexDochioiu commented Dec 1, 2025

Uh oh!

AlexDochioiu commented Dec 16, 2025

Uh oh!

stuartmorgan-g commented Dec 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AlexDochioiu commented Nov 30, 2025 •

edited

Loading