Fix TR background color doesn't fill the whole row (#1049)

Author: daohoangson

demo_app/test/table/row_color.png

@@ -10,6 +10,7 @@ part 'data/build_bits.dart';
 part 'data/css.dart';
 part 'data/html_style.dart';
 part 'data/image.dart';
+part 'data/lockable_list.dart';
 part 'data/normal_line_height.dart';
 
 /// A collection of style's key and value pairs.

packages/core/lib/src/core_data.dart

@@ -190,7 +190,7 @@ abstract class BuildTree extends BuildBit {
   /// - `HtmlWidget.customStylesBuilder`
   /// - [BuildOp.defaultStyles]
   /// - Attribute `style` of [dom.Element]
-  Iterable<css.Declaration> get styles;
+  LockableList<css.Declaration> get styles;
 
   /// Adds an inline style.
   @Deprecated('Use BuildOp.defaultStyles instead.')

packages/core/lib/src/data/build_bits.dart

@@ -0,0 +1,19 @@
+part of '../core_data.dart';
+
+/// A collection of values, that may be locked to prevent modification.
+abstract class LockableList<T> extends Iterable<T> {
+  /// Returns `true` if this list should not be modified.
+  bool get isLocked;
+
+  /// Adds [value] to the end of this list.
+  void add(T value);
+
+  /// Appends all objects of [iterable] to the end of this list.
+  void addAll(Iterable<T> iterable);
+
+  /// Inserts [element] at position [index] in this list.
+  void insert(int index, T element);
+
+  /// Inserts all objects of [iterable] at position [index] in this list.
+  void insertAll(int index, Iterable<T> iterable);
+}

packages/core/lib/src/data/lockable_list.dart

@@ -30,7 +30,7 @@ class CoreBuildTree extends BuildTree {
   final _isInlines = <bool>[];
   final BuildTree? _parent;
   final Iterable<_CoreBuildOp> _parentOps;
-  final _styles = <css.Declaration>[];
+  final _styles = _LockableDeclarations();
 
   CoreBuildTree._({
     this.customStylesBuilder,
@@ -67,7 +67,7 @@ class CoreBuildTree extends BuildTree {
   BuildTree get parent => _parent!;
 
   @override
-  Iterable<css.Declaration> get styles => _styles;
+  LockableList<css.Declaration> get styles => _styles;
 
   @override
   void operator []=(String key, String value) {
@@ -159,7 +159,12 @@ class CoreBuildTree extends BuildTree {
 
   @override
   css.Declaration? getStyle(String property) {
-    for (final style in _styles.reversed) {
+    final values = _styles._values;
+    if (values == null) {
+      return null;
+    }
+
+    for (final style in values.reversed) {
       if (style.property == property) {
         // TODO: add support for `!important`
         // https://github.com/daohoangson/flutter_widget_from_html/issues/773
@@ -308,8 +313,12 @@ class CoreBuildTree extends BuildTree {
       _styles.addAll(elementStyles);
     }
 
-    for (final style in _styles) {
-      wf.parseStyle(this, style);
+    _styles._isLocked = true;
+    final values = _styles._values;
+    if (values != null) {
+      for (final style in values.toList(growable: false)) {
+        wf.parseStyle(this, style);
+      }
     }
 
     wf.parseStyleDisplay(this, getStyle(kCssDisplay)?.term);
@@ -381,6 +390,46 @@ class _CoreBuildOp {
   }
 }
 
+class _LockableDeclarations extends LockableList<css.Declaration> {
+  var _isLocked = false;
+  List<css.Declaration>? _values;
+
+  @override
+  bool get isLocked => _isLocked;
+
+  @override
+  Iterator<css.Declaration> get iterator =>
+      _values?.iterator ?? const <css.Declaration>[].iterator;
+
+  @override
+  void add(css.Declaration value) {
+    assert(!_isLocked, 'Adding after being locked is undeterministic.');
+    _values ??= [];
+    _values?.add(value);
+  }
+
+  @override
+  void addAll(Iterable<css.Declaration> iterable) {
+    assert(!_isLocked, 'Adding after being locked is undeterministic.');
+    _values ??= [];
+    _values?.addAll(iterable);
+  }
+
+  @override
+  void insert(int index, css.Declaration element) {
+    assert(!_isLocked, 'Inserting after being locked is undeterministic.');
+    _values ??= [];
+    _values?.insert(index, element);
+  }
+
+  @override
+  void insertAll(int index, Iterable<css.Declaration> iterable) {
+    assert(!_isLocked, 'Inserting after being locked is undeterministic.');
+    _values ??= [];
+    _values?.insertAll(index, iterable);
+  }
+}
+
 class _WidgetPlaceholderDefault extends StatelessWidget
     implements
         // ignore: avoid_implementing_value_types

packages/core/lib/src/internal/core_build_tree.dart

@@ -338,6 +338,7 @@ class _TagTableRow {
 
   _TagTableRow() {
     _rowOp = BuildOp.v1(
+      alwaysRenderBlock: true,
       debugLabel: kTagTableRow,
       onChild: _onRowChild,
       priority: Priority.tagTableRow,
@@ -358,6 +359,11 @@ class _TagTableRow {
       return;
     }
 
+    for (final style in rowTree.styles) {
+      // forward all row styles to its cells, this is quite dangerous...
+      cellTree.styles.add(style);
+    }
+
     _registerCellOp(cellTree);
   }
 

packages/core/lib/src/internal/ops/tag_table.dart

@@ -0,0 +1,134 @@
+import 'package:csslib/parser.dart' as css;
+import 'package:csslib/visitor.dart' as css;
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter_widget_from_html_core/flutter_widget_from_html_core.dart';
+import 'package:flutter_widget_from_html_core/src/internal/core_build_tree.dart';
+import 'package:html/dom.dart' as dom;
+import 'package:html/parser.dart' as parser;
+
+void main() {
+  group('CoreBuildTree', () {
+    group('styles', () {
+      test('is unlocked initially', () {
+        final tree = _newTree();
+        expect(tree.styles.isLocked, isFalse);
+      });
+
+      test('is unlocked in onChild', () {
+        final list = <bool>[];
+        final tree = _newTree(
+          wf: _WidgetFactory(
+            onParse: (tree) => tree.register(
+              BuildOp.v1(
+                onChild: (_, subTree) => list.add(subTree.styles.isLocked),
+              ),
+            ),
+          ),
+        );
+        tree.addBitsFromNodes(_parseHtml('<div><span>Foo</span></div>'));
+        expect(list, equals([false]));
+      });
+
+      test('is locked in parseStyle', () {
+        final list = <bool>[];
+        final tree = _newTree(
+          wf: _WidgetFactory(
+            onParseStyle: (tree) => list.add(tree.styles.isLocked),
+          ),
+        );
+        tree.addBitsFromNodes(_parseHtml('<div>Foo</div>'));
+        expect(list, equals([true]));
+      });
+
+      group('modifications', () {
+        final red = _parseCss('color: red').first;
+        final green = _parseCss('color: green').first;
+        final blue = _parseCss('color: blue').first;
+
+        test('add value', () {
+          final tree = _newTree();
+          tree.styles.add(red);
+          expect(tree.styles.toList(), [red]);
+        });
+
+        test('add all values', () {
+          final tree = _newTree();
+          tree.styles.addAll([red, green]);
+          expect(tree.styles.toList(), [red, green]);
+        });
+
+        test('insert value', () {
+          final tree = _newTree();
+          tree.styles.insert(0, red);
+          expect(tree.styles.toList(), [red]);
+        });
+
+        test('insert all values', () {
+          final tree = _newTree();
+          tree.styles.insertAll(0, [red, green]);
+          expect(tree.styles.toList(), [red, green]);
+        });
+
+        test('insert into the middle', () {
+          final tree = _newTree();
+          tree.styles.addAll([red, blue]);
+          tree.styles.insert(1, green);
+          expect(tree.styles.toList(), [red, green, blue]);
+        });
+
+        test('throw error while being locked', () {
+          Object? addError;
+          final tree = _newTree(
+            wf: _WidgetFactory(
+              onParseStyle: (tree) {
+                try {
+                  tree.styles.add(red);
+                } catch (e) {
+                  addError = e;
+                }
+              },
+            ),
+          );
+          tree.addBitsFromNodes(_parseHtml('<div>Foo</div>'));
+          expect(addError, isAssertionError);
+        });
+      });
+    });
+  });
+}
+
+CoreBuildTree _newTree({
+  WidgetFactory? wf,
+}) =>
+    CoreBuildTree.root(
+      styleBuilder: HtmlStyleBuilder(),
+      wf: wf ?? _WidgetFactory(),
+    );
+
+List<css.Declaration> _parseCss(String input) =>
+    css.parse('*{$input}').collectDeclarations();
+
+dom.NodeList _parseHtml(String html) =>
+    parser.HtmlParser(html, parseMeta: false).parseFragment().nodes;
+
+class _WidgetFactory extends WidgetFactory {
+  final void Function(BuildTree)? onParse;
+  final void Function(BuildTree)? onParseStyle;
+
+  _WidgetFactory({
+    this.onParse,
+    this.onParseStyle,
+  });
+
+  @override
+  void parse(BuildTree tree) {
+    onParse?.call(tree);
+    super.parse(tree);
+  }
+
+  @override
+  void parseStyle(BuildTree tree, css.Declaration style) {
+    onParseStyle?.call(tree);
+    super.parseStyle(tree, style);
+  }
+}

packages/core/test/src/internal/core_build_tree_test.dart

@@ -549,8 +549,10 @@ Future<void> main() async {
       final explained = await explain(tester, html);
       expect(explained, equals('[widget0]'));
     });
+  });
 
-    testWidgets('#171: background-color', (WidgetTester tester) async {
+  group('background', () {
+    testWidgets('#171: cell color', (WidgetTester tester) async {
       // https://github.com/daohoangson/flutter_widget_from_html/issues/171
       const html = '<table><tr>'
           '<td style="background-color: #f00">Foo</td>'
@@ -569,6 +571,59 @@ Future<void> main() async {
         ),
       );
     });
+
+    testWidgets('#1028: row color', (WidgetTester tester) async {
+      // https://github.com/daohoangson/flutter_widget_from_html/issues/1028
+      const html = '<table><tr style="background-color: #f00">'
+          '<td>Foo</td><td>Bar</td>'
+          '</tr></table>';
+      final explained = await explain(tester, html);
+      expect(
+        explained,
+        equals(
+          '[HtmlTable:children='
+          '[HtmlTableCell:child='
+          '[Container:bg=#FFFF0000,child='
+          '[Padding:(1,1,1,1),child='
+          '[Align:alignment=centerLeft,widthFactor=1.0,child='
+          '[RichText:(:Foo)]'
+          ']]]],'
+          '[HtmlTableCell:child='
+          '[Container:bg=#FFFF0000,child='
+          '[Padding:(1,1,1,1),child='
+          '[Align:alignment=centerLeft,widthFactor=1.0,child='
+          '[RichText:(:Bar)]'
+          ']]]]'
+          ']',
+        ),
+      );
+    });
+
+    testWidgets('overwrites row color', (WidgetTester tester) async {
+      const html = '<table><tr style="background-color: #f00">'
+          '<td>Foo</td><td style="background-color: #0f0">Bar</td>'
+          '</tr></table>';
+      final explained = await explain(tester, html);
+      expect(
+        explained,
+        equals(
+          '[HtmlTable:children='
+          '[HtmlTableCell:child='
+          '[Container:bg=#FFFF0000,child='
+          '[Padding:(1,1,1,1),child='
+          '[Align:alignment=centerLeft,widthFactor=1.0,child='
+          '[RichText:(:Foo)]'
+          ']]]],'
+          '[HtmlTableCell:child='
+          '[Container:bg=#FF00FF00,child='
+          '[Padding:(1,1,1,1),child='
+          '[Align:alignment=centerLeft,widthFactor=1.0,child='
+          '[RichText:(:Bar)]'
+          ']]]]'
+          ']',
+        ),
+      );
+    });
   });
 
   testWidgets('renders display: table', (WidgetTester tester) async {
@@ -927,6 +982,30 @@ Future<void> main() async {
     <td valign="baseline"><div style="padding: 10px">$multiline</div></td>
     <td valign="baseline">Foo</td>
   </tr>
+</table>''',
+              'row_color': '''
+<!-- https://github.com/daohoangson/flutter_widget_from_html/issues/1028 -->
+<table style="border-collapse: collapse;">
+  <tr>
+    <th>First Name</th>
+    <th>Last Name</th>
+    <th>Points</th>
+  </tr>
+  <tr style="background-color: #f2f2f2;">
+    <td>Jill</td>
+    <td>Smith</td>
+    <td>50</td>
+  </tr>
+  <tr>
+    <td>Eve</td>
+    <td>Jackson</td>
+    <td>94</td>
+  </tr>
+  <tr style="background-color: #f2f2f2;">
+    <td>Adam</td>
+    <td>Johnson</td>
+    <td>67</td>
+  </tr>
 </table>''',
               'rtl': '''
 <table dir="rtl">