diff --git a/.fvmrc b/.fvmrc index 19e8577..049b27d 100644 --- a/.fvmrc +++ b/.fvmrc @@ -1,3 +1,3 @@ { "flutter": "3.41.6" -} \ No newline at end of file +} diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 2c2ffa6..baea198 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -44,7 +44,6 @@ jobs: # TODO clean up the dart analyzer info stuff # dart analyze --fatal-infos dart analyze - dart run custom_lint - name: Run all tests and generate coverage information in coverage/lcov.info run: flutter test --coverage diff --git a/lib/main.dart b/lib/main.dart index 3b9d988..4c267de 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,4 +1,5 @@ import 'package:app4training/l10n/generated/app_localizations.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:app4training/routes/routes.dart'; @@ -8,8 +9,121 @@ import 'data/app_language.dart'; import 'data/globals.dart'; import 'design/theme.dart'; +/// Installs a global [FlutterError.onError] wrapper that suppresses the +/// known non-fatal framework assertions produced by `flutter_html` / +/// `flutter_html_table` when rendering pages that contain tables, and +/// forwards everything else unchanged to the previously-installed +/// handler (typically [FlutterError.presentError]). +/// +/// Four assertion variants have been observed in the wild, all +/// originating from the `LayoutBuilder` / `LayoutGrid` / `WidgetSpan` +/// composition that `TableHtmlExtension.build` constructs for `` +/// rendering: +/// +/// 1. Paint-time `RenderBox was not laid out` — fires from +/// `PipelineOwner.flushPaint` → `RenderCSSBox.paint` → +/// `RenderDecoratedBox.paint` → `RenderBox.size`. Stack contains +/// `flutter_html/` and `flutter_layout_grid/` frames. +/// +/// 2. Semantics-pass `RenderBox was not laid out` — fires from +/// `PipelineOwner.flushSemantics` → +/// `_SemanticsGeometry.computeChildGeometry` → +/// `RenderBox.semanticBounds` → `RenderBox.size`. Reported via +/// `SchedulerBinding._invokeFrameCallback` with +/// `library: 'scheduler library'`. The stack is **pure framework +/// code** with no package frames, because the semantics walk +/// traverses the `_RenderObjectSemantics` tree directly without +/// going through widget code — so matching on package paths alone +/// would miss this variant. +/// +/// 3. Layout-time `RenderBox.size accessed in ...computeDryBaseline` +/// — fires from `RenderParagraph.performLayout` → +/// `layoutInlineChildren` → child `performLayout` reading `.size` +/// during a dry-baseline computation. Stack contains +/// `flutter_html/src/css_box_widget.dart` frames. +/// +/// 4. Layout-time `'child!.hasSize' is not true` — fires from +/// `RenderAligningShiftedBox.alignChild` while laying out a +/// `Container` constructed in `flutter_html_table.dart:261`. +/// Stack contains `flutter_html/` and `flutter_layout_grid/` +/// frames. +/// +/// All four are non-fatal: the page renders, users can interact +/// normally, selection still works outside tables. They flood logs +/// (hundreds per page load) which drowns out any real errors, which is +/// why we filter them. +/// +/// Why this lives here and not in the widget layer: +/// 1. The assertions are not reproducible in `flutter_test`, even +/// with `tester.ensureSemantics()` + a phone-sized surface, so we +/// cannot regression-test a widget-level fix; +/// 2. The root cause is inside third-party package code +/// (`flutter_html_table` 3.0.0). Wrapping tables in +/// `SelectionContainer.disabled` (mirroring Flutter's own +/// `raw_tooltip.dart:813-815` pattern) did not help and also +/// broke text selection across the whole page; +/// 3. The real fix is a newer upstream release. Until then this +/// filter keeps the logs usable. +/// +/// Matching strategy — an error is suppressed only if BOTH: +/// (a) the exception message contains one of the four known +/// signatures (`"RenderBox was not laid out"`, `"computeDryBaseline"`, +/// `"renderBoxDoingDryBaseline"`, or `"'child!.hasSize'"`), AND +/// (b) the stack either passes through `flutter_html/` or +/// `flutter_layout_grid/` package code (covers variants 1, 3, 4) +/// OR contains `"flushSemantics"` (covers variant 2, whose stack +/// is entirely framework code). +/// +/// Any error that doesn't satisfy BOTH conditions is forwarded to the +/// previous handler unchanged, so unrelated regressions are not masked. +/// +/// On the first suppression per app run we emit a single breadcrumb via +/// [debugPrint] so developers inspecting logs can see that the filter +/// is active. Subsequent suppressions are silent to avoid log spam. +/// +/// See also: `Task Progress.md` §5 for the full investigation history, +/// including the diagnostic logging session that identified all four +/// variants. +void _installHtmlTableSemanticsFilter() { + final FlutterExceptionHandler? previousHandler = FlutterError.onError; + var suppressedBreadcrumbEmitted = false; + FlutterError.onError = (FlutterErrorDetails details) { + final String exceptionText = details.exception.toString(); + final String stackText = details.stack?.toString() ?? ''; + final bool exceptionMatchesKnownSignature = + exceptionText.contains('RenderBox was not laid out') || + exceptionText.contains('computeDryBaseline') || + exceptionText.contains('renderBoxDoingDryBaseline') || + exceptionText.contains("'child!.hasSize'"); + final bool stackMatchesHtmlOrSemanticsPath = + stackText.contains('flutter_html/') || + stackText.contains('flutter_layout_grid/') || + stackText.contains('flushSemantics'); + final bool isKnownHtmlTableAssertion = + exceptionMatchesKnownSignature && stackMatchesHtmlOrSemanticsPath; + if (isKnownHtmlTableAssertion) { + if (!suppressedBreadcrumbEmitted) { + suppressedBreadcrumbEmitted = true; + debugPrint( + 'Suppressing known flutter_html_table non-fatal framework ' + 'assertions (see Task Progress.md §5 and main.dart ' + '_installHtmlTableSemanticsFilter for details). Subsequent ' + 'suppressions are silent.', + ); + } + return; + } + if (previousHandler != null) { + previousHandler(details); + } else { + FlutterError.presentError(details); + } + }; +} + void main() async { WidgetsFlutterBinding.ensureInitialized(); + _installHtmlTableSemanticsFilter(); final prefs = await SharedPreferences.getInstance(); final packageInfo = await PackageInfo.fromPlatform(); diff --git a/lib/widgets/html_view.dart b/lib/widgets/html_view.dart index 7763718..232cf03 100644 --- a/lib/widgets/html_view.dart +++ b/lib/widgets/html_view.dart @@ -12,69 +12,139 @@ class HtmlView extends StatelessWidget { /// left-to-right or right-to-left (LTR / RTL)? final TextDirection direction; + const HtmlView(this.content, this.direction, {super.key}); @override Widget build(BuildContext context) { return Padding( - padding: const EdgeInsets.all(8), - child: SingleChildScrollView( - child: Column( + padding: const EdgeInsets.all(8), + child: SingleChildScrollView( + child: Column( children: [ SelectionArea( - child: Directionality( - textDirection: direction, -// child: Html( -// data: content, - child: Html.fromDom( - document: sanitize( - content, - MediaQuery.of(context).platformBrightness == - Brightness.dark), - extensions: const [TableHtmlExtension()], - style: { - "body": Style(fontSize: FontSize(15)), - "table": Style( - // set table width, otherwise they're broken - width: Width( - MediaQuery.of(context).size.width - 50)), - "td": Style( - padding: const EdgeInsets.fromLTRB(5, 3, 5, 3) - .htmlPadding), - "th": Style( - textAlign: TextAlign.center, - verticalAlign: VerticalAlign.top), - "h1": Style( - margin: - Margins(top: Margin(0), bottom: Margin(0))), - "h2": Style( - margin: - Margins(top: Margin(12), bottom: Margin(5))), - "h3": Style( - margin: - Margins(top: Margin(10), bottom: Margin(3))), - "li": Style( - margin: - Margins(top: Margin(3), bottom: Margin(3))), - "p": Style( - margin: - Margins(top: Margin(3), bottom: Margin(3))), - "ul": Style( - margin: - Margins(top: Margin(0), bottom: Margin(0))), -// TODO: reduce left padding/margin of
  • items -// But this doesn't seem to work in flutter_html-3.0.0-beta2 -/* "li": Style( - padding: HtmlPaddings.zero, margin: Margins.zero) */ - }, - onAnchorTap: (url, _, __) { - debugPrint("Link tapped: $url"); - if (url != null) { - Navigator.pushNamed(context, '/view$url'); - } - }))) + child: Directionality( + textDirection: direction, + // child: Html( + // data: content, + child: Html.fromDom( + document: sanitize( + content, + MediaQuery.of(context).platformBrightness == + Brightness.dark, + ), + extensions: [ + // Order matters: TagWrapExtension must come BEFORE + // TableHtmlExtension so it matches
    • first + // during the preparing step. It then delegates the + // inner build to TableHtmlExtension via + // prepareFromExtension(extensionsToIgnore: {this}) + // and wraps the resulting table widget in a + // horizontal scroll view. Without this ordering, + // TableHtmlExtension wins the match and the wrap + // is never applied — leaving wide tables to + // overflow or hit "RenderBox was not laid out". + // + // Known remaining issue: on real devices the + // semantics pass logs a non-fatal per-frame + // "RenderBox was not laid out: RenderParagraph" + // assertion during PipelineOwner.flushSemantics, + // originating deep inside the + // LayoutGrid/LayoutBuilder cell tree built by + // TableHtmlExtension. Wrapping the table in + // SelectionContainer.disabled was tried as a + // workaround (mirroring raw_tooltip.dart:813-815) + // but did NOT fix the assertion and broke text + // selection across the whole page, so it was + // reverted. Page still renders fine; the spam is + // cosmetic in logs. + TagWrapExtension( + tagsToWrap: const {'table'}, + builder: (child) => _HorizontalTableScroll(child: child), + ), + const TableHtmlExtension(), + ], + style: { + "body": Style(fontSize: FontSize(15)), + "td": Style( + padding: + const EdgeInsets.fromLTRB(5, 3, 5, 3).htmlPadding, + ), + "th": Style( + textAlign: TextAlign.center, + verticalAlign: VerticalAlign.top, + ), + "h1": Style( + margin: Margins(top: Margin(0), bottom: Margin(0)), + ), + "h2": Style( + margin: Margins(top: Margin(12), bottom: Margin(5)), + ), + "h3": Style( + margin: Margins(top: Margin(10), bottom: Margin(3)), + ), + "li": Style( + margin: Margins(top: Margin(3), bottom: Margin(3)), + padding: HtmlPaddings.zero, + ), + "p": Style( + margin: Margins(top: Margin(3), bottom: Margin(3)), + ), + "ul": Style( + margin: Margins(top: Margin(0), bottom: Margin(0)), + ), + }, + onAnchorTap: (url, _, __) { + debugPrint("Link tapped: $url"); + if (url != null) { + Navigator.pushNamed(context, '/view$url'); + } + }, + ), + ), + ), ], - ))); + ), + ), + ); + } +} + +/// Wraps a rendered `
    ` in a horizontal scroll view with an +/// always-visible scrollbar. An explicit [ScrollController] is needed +/// because the scroll view sits inside a `WidgetSpan` (via +/// [TagWrapExtension]) where [PrimaryScrollController] does not apply. +/// [Scrollbar.thumbVisibility] is forced on so users can see at a glance +/// when a table extends beyond the screen and needs to be scrolled. +class _HorizontalTableScroll extends StatefulWidget { + const _HorizontalTableScroll({required this.child}); + + final Widget child; + + @override + State<_HorizontalTableScroll> createState() => _HorizontalTableScrollState(); +} + +class _HorizontalTableScrollState extends State<_HorizontalTableScroll> { + final ScrollController _controller = ScrollController(); + + @override + void dispose() { + _controller.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + return Scrollbar( + controller: _controller, + thumbVisibility: true, + child: SingleChildScrollView( + controller: _controller, + scrollDirection: Axis.horizontal, + child: widget.child, + ), + ); } } @@ -142,6 +212,19 @@ htmldom.Document sanitize(String inputHtml, bool isDarkMode) { for (var element in dom.querySelectorAll('th')) { element.attributes.remove('style'); } + // e.g. Time_with_God has
    . flutter_html 3.0.0 + // does NOT resolve percent widths against the containing block (see + // CssBoxWidget._computeSize and Normalize.normalize in + // flutter_html-3.0.0/lib/src/css_box_widget.dart): it uses the raw + // numeric value regardless of unit, so `width: 100%` is interpreted as + // literally 100 px and the table overflows by hundreds of pixels. + // Strip style and width attributes from
    so it falls back to + // Width.auto() and our horizontal-scroll wrapper can size the table + // to its intrinsic content width. + for (var element in dom.querySelectorAll('table')) { + element.attributes.remove('style'); + element.attributes.remove('width'); + } // For all worksheets with subtitles (e.g. "Overcoming Colored Lenses"): // Replace

    ...

    @@ -157,7 +240,7 @@ htmldom.Document sanitize(String inputHtml, bool isDarkMode) { // For God's Story (five fingers): // Remove
    0.7.9, but flutter_test pins test_api 0.7.9 mocktail: ^1.0.3 file: ^7.0.0 shared_preferences: ^2.2.0 @@ -68,6 +68,10 @@ dev_dependencies: # rules and activating additional ones. flutter_lints: ^6.0.0 full_coverage: ^1.0.0 + # custom_lint removed: custom_lint 0.8.1 requires analyzer ^8, but + # riverpod_lint >=3.1.1 requires analyzer ^9. riverpod_lint 3.1.3 + # migrated from custom_lint to analysis_server_plugin, so custom_lint + # is no longer needed here. riverpod_lint: ^3.1.3 integration_test: sdk: flutter diff --git a/test/assets-en/html-en-main/Time_with_God.html b/test/assets-en/html-en-main/Time_with_God.html new file mode 100644 index 0000000..17c8316 --- /dev/null +++ b/test/assets-en/html-en-main/Time_with_God.html @@ -0,0 +1,87 @@ +

    Time with God

    +

    If we want to know a person, we need to have regular contact with them. It’s the same in our relationship with God: To get to know Him better, we need to make the time to spend with Him. +

    +

    The purpose of our time with God

    + +

    Examples from the Bible

    +

    Look up the following Bible verses and fill out the table: Which person is it talking about? When, where and how exactly does this person spend time with God? +

    +
    + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    Verse +Person +Time +Place +What exactly? +
    Psalms 5:3 +David +in the morning +? +praying and waiting for answer +
    Daniel 6:11 +
    Mark 1:35 +
    Luke 6:12 +
    Acts 10:9 +
    Acts 16:25 +
    +

    Tools and suggestions for our time with God

    + +

    My commitment for my time with God

    +

    Time: +

    +
    +

    Place: +

    +
    +

    Plan: +

    +
    +


    +


    +

    \ No newline at end of file diff --git a/test/html_view_table_test.dart b/test/html_view_table_test.dart new file mode 100644 index 0000000..bb00a9c --- /dev/null +++ b/test/html_view_table_test.dart @@ -0,0 +1,221 @@ +import 'dart:io'; + +import 'package:app4training/widgets/html_view.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +/// Reproduces the "RenderBox was not laid out" crashes reported on pages +/// like Essentials > Time With God that contain tables. Only goal here: +/// make sure HtmlView can render common table shapes without throwing. +void main() { + Future pump(WidgetTester tester, String body) async { + await tester.pumpWidget(MaterialApp( + home: Scaffold( + body: HtmlView( + '$body', + TextDirection.ltr, + ), + ), + )); + await tester.pumpAndSettle(); + } + + testWidgets('renders a simple 2x2 table', (tester) async { + await pump(tester, ''' + + + +
    AB
    12
    +'''); + expect(tester.takeException(), isNull); + }); + + testWidgets('wraps each in a visible horizontal Scrollbar', + (tester) async { + // Guards the UX affordance: users need a visible scroll thumb to + // discover that wide tables are horizontally scrollable. + await pump(tester, ''' +
    + + +
    H1H2
    12
    + + + +
    Other
    x
    +'''); + expect(tester.takeException(), isNull); + // Expect one Scrollbar per table. The outer page scroll is a + // SingleChildScrollView without a Scrollbar, so every Scrollbar found + // here belongs to a _HorizontalTableScroll wrapper. + expect(find.byType(Scrollbar), findsNWidgets(2)); + // And the scroll direction inside each wrapper must be horizontal. + final horizontalScrolls = tester + .widgetList(find.byType(SingleChildScrollView)) + .where((w) => w.scrollDirection == Axis.horizontal) + .toList(); + expect(horizontalScrolls.length, 2); + }); + + testWidgets('renders a table with long non-breaking cell content', (tester) async { + // Long single token that cannot wrap — this is the kind of content that + // causes layout problems on some translations of Time With God. + await pump(tester, ''' + + + + + + +
    HeadingAnother heading
    Supercalifragilisticexpialidocious_long_word_that_cannot_wrap_naturallyShort
    +'''); + expect(tester.takeException(), isNull); + }); + + testWidgets('renders a wide many-column table', (tester) async { + await pump(tester, ''' + + + + + + + + + + + + + + + +
    Col1Col2Col3Col4Col5Col6Col7Col8
    row 1 cell 1 with some textrow 1 cell 2 with some textrow 1 cell 3 with some textrow 1 cell 4 with some textrow 1 cell 5 with some textrow 1 cell 6 with some textrow 1 cell 7 with some textrow 1 cell 8 with some text
    +'''); + expect(tester.takeException(), isNull); + }); + + testWidgets('renders a table with th[style] (stripped by sanitize)', + (tester) async { + await pump(tester, ''' + + + + + + + + + + + +
    OneTwoThree
    Short cellMedium length cell content that wrapsSome other content here that is longer than the others
    +'''); + expect(tester.takeException(), isNull); + }); + + testWidgets('renders a table with empty cells (fill-in-the-blank)', + (tester) async { + // Real Time_with_God table shape: lots of empty cells for users to fill in. + await pump(tester, ''' + + + + + + + + + + + + + + + + + + + + + + +
    VersePersonTimePlaceWhat exactly?
    Psalms 5:3Davidin the morning?praying and waiting for answer
    Daniel 6:11
    Mark 1:35
    Luke 6:12
    Acts 10:9
    Acts 16:25
    +'''); + expect(tester.takeException(), isNull); + }); + + testWidgets('renders a nested inside a
  • (Time_with_God shape)', + (tester) async { + // This is the exact shape from Time_with_God line 70: a table inside a + // list item inside a ul. It uses a
    before the nested table. + await pump(tester, ''' +
      +
    • Bible: Read a passage and think about it. Use the head-heart-hands questions:
      +
    • + + + + + + + + + + + + + + +
    Head-32.pngHead: What do I learn here?
    Heart-32.pngHeart: What touches my heart?
    Hands-32.pngHands: How can I apply this?
    + +
  • Place: Choose a quiet place.
    • +
  • Time: Find the best time.
    • + +'''); + expect(tester.takeException(), isNull); + }); + + testWidgets('renders the real Time_with_God fixture end-to-end', + (tester) async { + // Pumping the exact page content that was crashing in production. + // Critically: + // 1. enable semantics — on a real device the semantics pass walks the + // render tree and trips `RenderBox.size` assertions on any unlaid + // RenderParagraph; without this handle flutter_test skips + // flushSemantics and hides the bug; + // 2. use a phone-sized surface — the default 800x600 flutter_test + // surface is wide enough that the Time_with_God table fits without + // horizontal overflow, so the horizontal-scroll / LayoutGrid path + // that triggers the assertion on real devices never runs. + final semantics = tester.ensureSemantics(); + try { + await tester.binding.setSurfaceSize(const Size(390, 844)); + final html = File('test/assets-en/html-en-main/Time_with_God.html') + .readAsStringSync(); + await pump(tester, html); + expect(tester.takeException(), isNull); + } finally { + await tester.binding.setSurfaceSize(null); + semantics.dispose(); + } + }); + + testWidgets('renders a table wrapped in the standard page chrome', + (tester) async { + // Mimics the structure of a real Time With God page: headings, + // paragraphs, and a table together. + await pump(tester, ''' +

    Time with God

    +

    Introduction paragraph describing the topic.

    +

    Three different voices

    + + + + + +
    VoiceSourceEffect
    YourselfYour own thoughtsSelf-centered
    EnemyThe enemyAccusing, fearful
    GodHoly SpiritLoving, convicting
    +

    Follow-up text after the table.

    +'''); + expect(tester.takeException(), isNull); + }); +}