Update currentColor handling. (#730)

* Update currentColor handling. Default currentColor to black, mirroring Chrome's behavior. Update callsites to parseColor so that they account for currentColor. Avoid setting a default currentColor on the root element when parsing. Avoid painting when Paint.color == null.
flutter · Jul 2, 2022 · 353c2cf · 353c2cf
1 parent 7d886d4
commit 353c2cf
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 37 deletions.
diff --git a/third_party/CHANGELOG.md b/third_party/CHANGELOG.md
@@ -1,5 +1,11 @@
 # CHANGES
 
+## 1.1.1+1
+
+- Fix regression introduced in 1.1.1
+- Update fix for fill/stroke inheritence when currentColor is specified in the
+  SVG but not in the theme.
+
 ## 1.1.1
 
 - Fix a bug introduced in 1.1.0 related to fill/stroke inheritence.

diff --git a/third_party/lib/avd.dart b/third_party/lib/avd.dart
@@ -1,5 +1,5 @@
 import 'dart:convert';
-import 'dart:typed_data';
+import 'dart:typed_data'; // ignore: unnecessary_import
 import 'dart:ui' show Picture;
 
 import 'package:flutter/foundation.dart';

diff --git a/third_party/lib/src/svg/parser_state.dart b/third_party/lib/src/svg/parser_state.dart
@@ -87,11 +87,10 @@ class _Elements {
 
     final String? id = parserState.attribute('id', def: '');
 
-    final Color? color =
-        parserState.parseColor(parserState.attribute('color', def: null)) ??
-            // Fallback to the currentColor from theme if no color is defined
-            // on the root SVG element.
-            parserState.theme.currentColor;
+    final Color? color = parserState.parseColor(
+      parserState.attribute('color', def: null),
+      currentColor: parserState.theme.currentColor,
+    );
 
     // TODO(dnfield): Support nested SVG elements. https://github.com/dnfield/flutter_svg/issues/132
     if (parserState._root != null) {
@@ -144,9 +143,10 @@ class _Elements {
       return null;
     }
     final DrawableParent parent = parserState.currentGroup!;
-    final Color? color =
-        parserState.parseColor(parserState.attribute('color', def: null)) ??
-            parent.color;
+    final Color? color = parserState.parseColor(
+            parserState.attribute('color', def: null),
+            currentColor: parent.color ?? parserState.theme.currentColor) ??
+        parent.color;
 
     final DrawableGroup group = DrawableGroup(
       parserState.attribute('id', def: ''),
@@ -164,9 +164,10 @@ class _Elements {
   static Future<void>? symbol(
       SvgParserState parserState, bool warningsAsErrors) {
     final DrawableParent parent = parserState.currentGroup!;
-    final Color? color =
-        parserState.parseColor(parserState.attribute('color', def: null)) ??
-            parent.color;
+    final Color? color = parserState.parseColor(
+            parserState.attribute('color', def: null),
+            currentColor: parent.color ?? parserState.theme.currentColor) ??
+        parent.color;
 
     final DrawableGroup group = DrawableGroup(
       parserState.attribute('id', def: ''),
@@ -240,7 +241,8 @@ class _Elements {
           def: '1',
         )!;
         final Color stopColor = parserState.parseColor(
-                getAttribute(parserState.attributes, 'stop-color')) ??
+                getAttribute(parserState.attributes, 'stop-color'),
+                currentColor: parent.color ?? parserState.theme.currentColor) ??
             parent.color ??
             colorBlack;
         colors.add(stopColor.withOpacity(parseDouble(rawOpacity)!));
@@ -1339,7 +1341,7 @@ class SvgParserState {
       );
       strokeColor = definitionPaint.color;
     } else {
-      strokeColor = parseColor(rawStroke);
+      strokeColor = parseColor(rawStroke, currentColor: currentColor);
     }
 
     final DrawablePaint paint = DrawablePaint(
@@ -1433,8 +1435,7 @@ class SvgParserState {
     Color? defaultFillColor,
     Color? currentColor,
   ) {
-    final Color? color = parseColor(rawFill) ??
-        currentColor ??
+    final Color? color = parseColor(rawFill, currentColor: currentColor) ??
         parentFillColor ??
         defaultFillColor;
 
@@ -1634,6 +1635,7 @@ class SvgParserState {
         ),
         decorationColor: parseColor(
           getAttribute(attributes, 'text-decoration-color', def: null),
+          currentColor: currentColor,
         ),
         decorationStyle: parseTextDecorationStyle(
           getAttribute(attributes, 'text-decoration-style', def: null),
@@ -1644,7 +1646,7 @@ class SvgParserState {
   }
 
   /// Converts a SVG Color String (either a # prefixed color string or a named color) to a [Color].
-  Color? parseColor(String? colorString) {
+  Color? parseColor(String? colorString, {Color? currentColor}) {
     if (colorString == null || colorString.isEmpty) {
       return null;
     }
@@ -1655,7 +1657,7 @@ class SvgParserState {
 
     if (colorString.toLowerCase() == 'currentcolor') {
       _compatibilityTester.usesCurrentColor = true;
-      return null;
+      return currentColor ?? theme.currentColor;
     }
 
     // handle hex colors e.g. #fff or #ffffff.  This supports #RRGGBBAA

diff --git a/third_party/lib/src/svg/theme.dart b/third_party/lib/src/svg/theme.dart
@@ -10,14 +10,14 @@ class SvgTheme {
   ///
   /// Defaults the [fontSize] to 14.
   const SvgTheme({
-    this.currentColor,
+    this.currentColor = const Color(0xFF000000),
     this.fontSize = 14,
     double? xHeight,
   }) : xHeight = xHeight ?? fontSize / 2;
 
   /// The default color applied to SVG elements that inherit the color property.
   /// See: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#currentcolor_keyword
-  final Color? currentColor;
+  final Color currentColor;
 
   /// The font size used when calculating em units of SVG elements.
   /// See: https://www.w3.org/TR/SVG11/coords.html#Units

diff --git a/third_party/lib/src/utilities/_http_io.dart b/third_party/lib/src/utilities/_http_io.dart
@@ -1,5 +1,5 @@
 import 'dart:io';
-import 'dart:typed_data';
+import 'dart:typed_data'; // ignore: unnecessary_import
 
 import 'package:flutter/foundation.dart';
 

diff --git a/third_party/lib/src/vector_drawable.dart b/third_party/lib/src/vector_drawable.dart
@@ -1288,12 +1288,12 @@ class DrawableShape implements DrawableStyleable {
       if (style.mask != null) {
         canvas.saveLayer(null, Paint());
       }
-      if (style.fill?.style != null) {
+      if (style.fill?.color != null) {
         assert(style.fill!.style == PaintingStyle.fill);
         canvas.drawPath(path, style.fill!.toFlutterPaint());
       }
 
-      if (style.stroke?.style != null) {
+      if (style.stroke?.color != null) {
         assert(style.stroke!.style == PaintingStyle.stroke);
         if (style.dashArray != null &&
             !identical(style.dashArray, DrawableStyle.emptyDashArray)) {

diff --git a/third_party/lib/svg.dart b/third_party/lib/svg.dart
@@ -1,6 +1,6 @@
 import 'dart:async';
 import 'dart:convert';
-import 'dart:typed_data';
+import 'dart:typed_data'; // ignore: unnecessary_import
 import 'dart:ui' show Picture;
 
 import 'package:flutter/foundation.dart';
@@ -780,8 +780,8 @@ class _SvgPictureState extends State<SvgPicture> {
     final SvgTheme? defaultSvgTheme = DefaultSvgTheme.of(context)?.theme;
     final TextStyle defaultTextStyle = DefaultTextStyle.of(context).style;
 
-    final Color? currentColor =
-        widget.theme?.currentColor ?? defaultSvgTheme?.currentColor;
+    final Color currentColor =
+        widget.theme?.currentColor ?? defaultSvgTheme?.currentColor ?? const Color(0xFF000000);
 
     final double fontSize = widget.theme?.fontSize ??
         defaultSvgTheme?.fontSize ??

diff --git a/third_party/pubspec.yaml b/third_party/pubspec.yaml
@@ -1,7 +1,7 @@
 name: flutter_svg
 description: An SVG rendering and widget library for Flutter, which allows painting and displaying Scalable Vector Graphics 1.1 files.
 homepage: https://github.com/dnfield/flutter_svg
-version: 1.1.1
+version: 1.1.1+1
 
 dependencies:
   flutter:

diff --git a/third_party/test/colors_svg_test.dart b/third_party/test/colors_svg_test.dart
@@ -44,8 +44,10 @@ void main() {
         const Color(0xFF6F2173));
     expect(testSvgParserState.parseColor('hsla(0,0%,100%, 0.0)'),
         const Color(0x00FFFFFF));
-    expect(testSvgParserState.parseColor('currentColor'), null);
-    expect(testSvgParserState.parseColor('currentcolor'), null);
+    expect(testSvgParserState.parseColor('currentColor'),
+        testSvgParserState.theme.currentColor);
+    expect(testSvgParserState.parseColor('currentcolor'),
+        testSvgParserState.theme.currentColor);
     expect(
         () => testSvgParserState.parseColor('invalid name'), throwsStateError);
   });

diff --git a/third_party/test/vector_drawable_test.dart b/third_party/test/vector_drawable_test.dart
@@ -2,7 +2,6 @@ import 'dart:typed_data';
 import 'dart:ui';
 
 import 'package:flutter_svg/flutter_svg.dart';
-import 'package:flutter_svg/src/svg/parser_state.dart';
 import 'package:flutter_test/flutter_test.dart';
 
 void main() {
@@ -117,7 +116,7 @@ void main() {
     recorder.endRecording();
   });
 
-  test('draws even if color is null', () async {
+  test('Does not draw if color is null', () async {
     final DrawableShape shape = DrawableShape(
       'test',
       Path()..addRect(const Rect.fromLTRB(0, 0, 50, 50)),
@@ -130,12 +129,8 @@ void main() {
     final PathRecordingCanvas canvas = PathRecordingCanvas();
     shape.draw(canvas, Rect.largest);
 
-    expect(canvas.paths.length, 2);
-    expect(canvas.paints.length, 2);
-    expect(canvas.paints.first.style, PaintingStyle.fill);
-    expect(canvas.paints.first.color, colorBlack);
-    expect(canvas.paints.last.style, PaintingStyle.stroke);
-    expect(canvas.paints.last.color, colorBlack);
+    expect(canvas.paths.length, 0);
+    expect(canvas.paints.length, 0);
   });
 }