Emit floating-point rgb() values as percentages (#2800)

Closes #2799

Author: nex3

CHANGELOG.md

@@ -1,4 +1,12 @@
-## 1.101.4-dev
+## 1.101.4
+
+* Avoid emitting `rgb()` or `rgba()` functions with non-percent decimal
+  channels. Older browsers only support integer values or (potentially decimal)
+  percentages for these functions, so in order to preserve
+  backwards-compatibility while retaining full precision for modern browsers,
+  legacy colors that contain at least one non-integer channel will now use
+  percentages for their channels (for example, `rgb(0%, 100%, 50%)` rather than
+  `rgb(0, 255, 127.5)`).
 
 * Fix a bug where the values of plain-CSS `if()` expressions were emitted using
   their `meta.inspect()` format rather than their CSS serialization format.

lib/src/visitor/serialize.dart

@@ -114,7 +114,10 @@ String serializeSelector(Selector selector, {bool inspect = false}) {
 final class _SerializeVisitor
     implements CssVisitor<void>, ValueVisitor<void>, SelectorVisitor<void> {
   /// A buffer that contains the CSS produced so far.
-  final SourceMapBuffer _buffer;
+  ///
+  /// This can be temporarily replaced to capture a particular chunk of
+  /// serialization to a string.
+  SourceMapBuffer _buffer;
 
   /// The current indentation of the CSS output.
   var _indentation = 0;
@@ -763,42 +766,17 @@ final class _SerializeVisitor
     // In compressed mode, emit colors in the shortest representation possible.
     if (_isCompressed) {
       var rgb = color.toSpace(ColorSpace.rgb);
-      if (opaque && _tryIntegerRgb(rgb)) return;
-
-      var red = _writeNumberToString(rgb.channel0);
-      var green = _writeNumberToString(rgb.channel1);
-      var blue = _writeNumberToString(rgb.channel2);
+      if (opaque && _tryHexOrNamedRgb(rgb)) return;
 
-      var hsl = color.toSpace(ColorSpace.hsl);
-      var hue = _writeNumberToString(hsl.channel0);
-      var saturation = _writeNumberToString(hsl.channel1);
-      var lightness = _writeNumberToString(hsl.channel2);
+      var rgbString = _capture(() => _writeRgb(rgb));
+      var hslString = _capture(() => _writeHsl(rgb.toSpace(ColorSpace.hsl)));
 
       // Add two characters for HSL for the %s on saturation and lightness.
-      if (red.length + green.length + blue.length <=
-          hue.length + saturation.length + lightness.length + 2) {
-        _buffer
-          ..write(opaque ? 'rgb(' : 'rgba(')
-          ..write(red)
-          ..writeCharCode($comma)
-          ..write(green)
-          ..writeCharCode($comma)
-          ..write(blue);
+      if (rgbString.length <= hslString.length + 2) {
+        _buffer.write(rgbString);
       } else {
-        _buffer
-          ..write(opaque ? 'hsl(' : 'hsla(')
-          ..write(hue)
-          ..writeCharCode($comma)
-          ..write(saturation)
-          ..write('%,')
-          ..write(lightness)
-          ..writeCharCode($percent);
-      }
-      if (!opaque) {
-        _buffer.writeCharCode($comma);
-        _writeNumber(color.alpha);
+        _buffer.write(hslString);
       }
-      _buffer.writeCharCode($rparen);
       return;
     }
 
@@ -852,7 +830,7 @@ final class _SerializeVisitor
   ///
   /// Otherwise, writes nothing and returns `false`. Assumes [value] is in the
   /// RGB space.
-  bool _tryIntegerRgb(SassColor rgb) {
+  bool _tryHexOrNamedRgb(SassColor rgb) {
     assert(rgb.space == ColorSpace.rgb);
     if (!_canUseHex(rgb)) return false;
 
@@ -898,11 +876,14 @@ final class _SerializeVisitor
     var opaque = fuzzyEquals(color.alpha, 1);
     var rgb = color.toSpace(ColorSpace.rgb);
     _buffer.write(opaque ? "rgb(" : "rgba(");
-    _writeNumber(rgb.channel('red'));
-    _buffer.write(_commaSeparator);
-    _writeNumber(rgb.channel('green'));
-    _buffer.write(_commaSeparator);
-    _writeNumber(rgb.channel('blue'));
+
+    if (!_tryIntegerRgbChannels(rgb)) {
+      _writeChannel(color.channel0 * 100 / 255, '%');
+      _buffer.write(_commaSeparator);
+      _writeChannel(color.channel1 * 100 / 255, '%');
+      _buffer.write(_commaSeparator);
+      _writeChannel(color.channel2 * 100 / 255, '%');
+    }
 
     if (!opaque) {
       _buffer.write(_commaSeparator);
@@ -912,6 +893,32 @@ final class _SerializeVisitor
     _buffer.writeCharCode($rparen);
   }
 
+  /// If [value]'s channels are all integers, writes them as such and returns
+  /// `true`.
+  ///
+  /// Otherwise, writes nothing and returns `false`. Assumes [value] is in the
+  /// RGB space.
+  bool _tryIntegerRgbChannels(SassColor rgb) {
+    assert(rgb.space == ColorSpace.rgb);
+
+    var red = _asInt(rgb.channel0);
+    if (red == null) return false;
+
+    var green = _asInt(rgb.channel1);
+    if (green == null) return false;
+
+    var blue = _asInt(rgb.channel2);
+    if (blue == null) return false;
+
+    _buffer.write(_removeExponent(red.toString()));
+    _buffer.write(_commaSeparator);
+    _buffer.write(_removeExponent(green.toString()));
+    _buffer.write(_commaSeparator);
+    _buffer.write(_removeExponent(blue.toString()));
+
+    return true;
+  }
+
   /// Writes [value] as an `hsl()` or `hsla()` function.
   void _writeHsl(SassColor color) {
     var opaque = fuzzyEquals(color.alpha, 1);
@@ -1143,38 +1150,31 @@ final class _SerializeVisitor
     }
   }
 
-  /// Like [_writeNumber], but returns a string rather than writing to
-  /// [_buffer].
-  String _writeNumberToString(double number) {
-    var buffer = NoSourceMapBuffer();
-    _writeNumber(number, buffer);
-    return buffer.toString();
-  }
-
   /// Writes [number] without exponent notation and with at most
   /// [SassNumber.precision] digits after the decimal point.
   ///
   /// The number is written to [buffer], which defaults to [_buffer].
-  void _writeNumber(double number, [SourceMapBuffer? buffer]) {
-    buffer ??= _buffer;
+  void _writeNumber(double number) {
+    if (!number.isFinite) {
+      visitCalculation(
+          SassCalculation.unsimplified('calc', [SassNumber(number)]));
+      return;
+    }
 
     // Dart always converts integers to strings in the obvious way, so all we
     // have to do is clamp doubles that are close to being integers.
-    if (fuzzyAsInt(number) case var integer?
-        // In inspect mode, we want to show the full precision of every number,
-        // so we only write them as integers when they're precisely equal.
-        when !_inspect || number == integer) {
+    if (_asInt(number) case var integer?) {
       // JS still uses exponential notation for integers, so we have to handle
       // it here.
-      buffer.write(_removeExponent(integer.toString()));
+      _buffer.write(_removeExponent(integer.toString()));
       return;
     }
 
     var text = _removeExponent(number.toString());
 
     // Write the number at full precision in inspect mode.
     if (_inspect) {
-      buffer.write(text);
+      _buffer.write(text);
       return;
     }
 
@@ -1185,11 +1185,22 @@ final class _SerializeVisitor
 
     if (canWriteDirectly) {
       if (_isCompressed && text.codeUnitAt(0) == $0) text = text.substring(1);
-      buffer.write(text);
+      _buffer.write(text);
       return;
     }
 
-    _writeRounded(text, buffer);
+    _writeRounded(text, _buffer);
+  }
+
+  /// If [number] is close enough to an integer, returns it as one.
+  ///
+  /// Normally, "close enough" includes fuzzy matching, but in inspect mode we
+  /// want to show the full precision of every number so "close enough" requires
+  /// literally being an integer.
+  int? _asInt(double number) {
+    if (_inspect) return fuzzyAsInt(number);
+    var rounded = number.round();
+    return rounded == number ? rounded : null;
   }
 
   /// If [text] is written in exponent notation, returns a string representation
@@ -1854,6 +1865,19 @@ final class _SerializeVisitor
   bool _isInvisible(CssNode node) =>
       !_inspect &&
       (_isCompressed ? node.isInvisibleHidingComments : node.isInvisible);
+
+  /// Runs [callback] without adding to [_buffer] and returns the text it would
+  /// have emitted.
+  String _capture(void Function() callback) {
+    var oldBuffer = _buffer;
+    _buffer = NoSourceMapBuffer();
+    try {
+      callback();
+      return _buffer.toString();
+    } finally {
+      _buffer = oldBuffer;
+    }
+  }
 }
 
 /// An enum of generated CSS styles.

pkg/sass-parser/CHANGELOG.md

@@ -1,4 +1,4 @@
-## 0.4.50-dev
+## 0.4.50
 
 * No user-visible changes.
 

pkg/sass-parser/package.json

@@ -1,6 +1,6 @@
 {
   "name": "sass-parser",
-  "version": "0.4.50-dev",
+  "version": "0.4.50",
   "description": "A PostCSS-compatible wrapper of the official Sass parser",
   "repository": "sass/dart-sass",
   "author": "Google Inc.",

pkg/sass_api/CHANGELOG.md

@@ -1,4 +1,4 @@
-## 17.7.4-dev
+## 17.7.4
 
 * No user-visible changes.
 

pkg/sass_api/pubspec.yaml

@@ -2,7 +2,7 @@ name: sass_api
 # Note: Every time we add a new Sass AST node, we need to bump the *major*
 # version because it's a breaking change for anyone who's implementing the
 # visitor interface(s).
-version: 17.7.4-dev
+version: 17.7.4
 description: Additional APIs for Dart Sass.
 homepage: https://github.com/sass/dart-sass
 

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.101.4-dev
+version: 1.101.4
 description: A Sass implementation in Dart.
 homepage: https://github.com/sass/dart-sass