From e4f34b8a29eb97a6a247356e5c806ad01b4a36fc Mon Sep 17 00:00:00 2001 From: Shubham Date: Mon, 13 Apr 2026 01:19:22 +0530 Subject: [PATCH 1/5] fix(ast): SyntaxTree.replaceNode TypeError on non-leaf parents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SyntaxTree.replaceNode` previously hit a runtime `_TypeError` on any mutation whose parent was a concrete subclass of `GreenNode`: type 'List' is not a subtype of type 'List' of 'newChildren' The naïve `posParent.children.map((c) => identical(c, pos) ? newNode : c?.value).toList()` builds a `List`, but every concrete subclass of `GreenNode` narrows the parameter of `updateChildren` via the `covariant` keyword: - EquationRowNode → List (non-null) - FracNode, AccentNode, etc. → List (typed, non-null) - MultiscriptsNode, SqrtNode, NaryOperatorNode → List (nullable) The covariant check fires at runtime whenever the static type differs from the concrete override's expected list element type — which is effectively always. Dispatch on the concrete parent type and materialise a correctly-typed list before calling `updateChildren`. Regression tests cover all three parent-type shapes plus root replacement. --- lib/src/ast/syntax_tree.dart | 51 +++++++++++++++-- test/replace_node_test.dart | 103 +++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 test/replace_node_test.dart diff --git a/lib/src/ast/syntax_tree.dart b/lib/src/ast/syntax_tree.dart index 3491b637..ddc6c822 100644 --- a/lib/src/ast/syntax_tree.dart +++ b/lib/src/ast/syntax_tree.dart @@ -53,10 +53,53 @@ class SyntaxTree { 'The replaced node is not the root of this tree but has no parent'); } return replaceNode( - posParent, - posParent.value.updateChildren(posParent.children - .map((child) => identical(child, pos) ? newNode : child?.value) - .toList(growable: false))); + posParent, _rebuildParent(posParent, pos, newNode)); + } + + /// Build a new green node equivalent to [posParent].value but with [pos]'s + /// green node replaced by [newNode]. Dispatches on the concrete parent type + /// because every subclass of [GreenNode] narrows the type of the list + /// argument passed to [updateChildren]: + /// - [EquationRowNode] → List (non-null) + /// - [FracNode] & friends → List (typed, non-null) + /// - [MultiscriptsNode] & friends → List (nullable) + /// + /// A naive `posParent.value.updateChildren(children.map(...).toList())` + /// builds a `List` which fails every covariant check at runtime. + /// We materialise the correctly-typed list here instead. + static GreenNode _rebuildParent( + SyntaxNode posParent, SyntaxNode pos, GreenNode newNode) { + final parentValue = posParent.value; + final rawChildren = posParent.children; + + if (parentValue is EquationRowNode) { + final newChildren = [ + for (final child in rawChildren) + identical(child, pos) ? newNode : child!.value, + ]; + return parentValue.updateChildren(newChildren); + } + + // Every other non-leaf parent type expects EquationRowNode children + // (either nullable or non-nullable). Build the correctly-typed list. + final nullableChildren = []; + var hasNulls = false; + for (final child in rawChildren) { + if (child == null) { + nullableChildren.add(null); + hasNulls = true; + } else if (identical(child, pos)) { + nullableChildren.add(newNode as EquationRowNode?); + if (newNode == null) hasNulls = true; + } else { + nullableChildren.add(child.value as EquationRowNode); + } + } + if (!hasNulls) { + return parentValue + .updateChildren(nullableChildren.cast()); + } + return parentValue.updateChildren(nullableChildren); } List findNodesAtPosition(int position) { diff --git a/test/replace_node_test.dart b/test/replace_node_test.dart new file mode 100644 index 00000000..1213a8e3 --- /dev/null +++ b/test/replace_node_test.dart @@ -0,0 +1,103 @@ +// Regression tests for SyntaxTree.replaceNode. +// +// Before the fix, replaceNode built a `List` and passed it to +// the concrete overrides of `updateChildren`, which narrow the parameter +// type in every subclass (EquationRowNode wants List, FracNode +// wants List, etc.). This produced a runtime _TypeError +// on virtually any in-tree mutation. These tests cover the three parent +// shapes exhaustively: an EquationRowNode parent, a FracNode parent, and a +// MultiscriptsNode parent (nullable-slot case). + +import 'package:flutter_math_fork/ast.dart'; +import 'package:flutter_math_fork/tex.dart'; +import 'package:flutter_test/flutter_test.dart'; + +SyntaxTree _parse(String tex) => + SyntaxTree(greenRoot: TexParser(tex, const TexParserSettings()).parse()); + +void main() { + group('SyntaxTree.replaceNode', () { + test('replaces a direct child of EquationRowNode', () { + final tree = _parse(r'x + y'); + final root = tree.root; + // Find the first non-null red child — on `x + y` it will be the + // SymbolNode for `x`. + final firstChild = root.children.firstWhere((c) => c != null)!; + expect(firstChild.value, isA()); + + final replacement = SymbolNode(symbol: 'z'); + final updated = tree.replaceNode(firstChild, replacement); + + // The root row should still have the same number of children, + // with the first one replaced. + expect(updated.greenRoot, isA()); + final newFirst = updated.greenRoot.children.first; + expect(newFirst, isA()); + expect((newFirst as SymbolNode).symbol, 'z'); + expect(updated.greenRoot.children.length, + tree.greenRoot.children.length); + }); + + test('replaces the numerator of a FracNode', () { + final tree = _parse(r'\frac{1}{2}'); + // Walk the red tree: root -> FracNode -> numerator (EquationRowNode) + final fracRed = tree.root.children.firstWhere((c) => c != null)!; + expect(fracRed.value, isA()); + final numeratorRed = fracRed.children.firstWhere((c) => c != null)!; + expect(numeratorRed.value, isA()); + + final replacement = EquationRowNode( + children: [SymbolNode(symbol: '9')], + ); + final updated = tree.replaceNode(numeratorRed, replacement); + + final newFrac = updated.greenRoot.children.first as FracNode; + final newNumLeaf = newFrac.numerator.children.first as SymbolNode; + expect(newNumLeaf.symbol, '9'); + // Denominator must be untouched. + final denomLeaf = newFrac.denominator.children.first as SymbolNode; + expect(denomLeaf.symbol, '2'); + }); + + test('replaces the superscript slot of a MultiscriptsNode (nullable)', () { + final tree = _parse(r'x^{2}'); + final multiRed = tree.root.children.firstWhere((c) => c != null)!; + expect(multiRed.value, isA()); + // Children of MultiscriptsNode are [base, sub, sup, presub, presup] — + // many of which are null. The non-null ones here should be base & sup. + final nonNullChildren = + multiRed.children.whereType().toList(); + expect(nonNullChildren.length, greaterThanOrEqualTo(2)); + + // Locate the sup slot — it's the one whose green value contains the `2`. + final supRed = nonNullChildren.firstWhere((c) { + final row = c.value as EquationRowNode; + return row.children.any( + (x) => x is SymbolNode && x.symbol == '2'); + }); + + final replacement = EquationRowNode( + children: [SymbolNode(symbol: '7')], + ); + final updated = tree.replaceNode(supRed, replacement); + + final newMulti = updated.greenRoot.children.first as MultiscriptsNode; + expect(newMulti.sup, isNotNull); + final newSupLeaf = newMulti.sup!.children.first as SymbolNode; + expect(newSupLeaf.symbol, '7'); + // Base is untouched. + final newBaseLeaf = newMulti.base.children.first as SymbolNode; + expect(newBaseLeaf.symbol, 'x'); + }); + + test('replacing the root wraps in an EquationRowNode', () { + final tree = _parse(r'\frac{1}{2}'); + final replacement = SymbolNode(symbol: 'q'); + final updated = tree.replaceNode(tree.root, replacement); + // A LeafNode replacement at the root must be wrapped. + expect(updated.greenRoot, isA()); + expect(updated.greenRoot.children.length, 1); + expect((updated.greenRoot.children.first as SymbolNode).symbol, 'q'); + }); + }); +} From 82989f1a763d87389a12e8adc5dcabd1dd458a30 Mon Sep 17 00:00:00 2001 From: Shubham Date: Mon, 13 Apr 2026 01:19:30 +0530 Subject: [PATCH 2/5] fix(widgets): preserve selection across ast mutations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MathController.ast = newTree` hard-reset the selection to `TextSelection.collapsed(offset: -1)` on every tree change. This made the caret unusable for any editable-math use case — every insertion, deletion, or structural transform would erase the cursor position. Call `sanitizeSelection(_ast, _selection)` instead, which clamps the existing selection to the new tree's valid range (exactly what the `selection =` setter already does). Selection is preserved as long as the new tree contains the old cursor position; otherwise it's clamped to the valid range instead of blown away. --- lib/src/widgets/controller.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/src/widgets/controller.dart b/lib/src/widgets/controller.dart index 6bff90c4..76c37c75 100644 --- a/lib/src/widgets/controller.dart +++ b/lib/src/widgets/controller.dart @@ -14,7 +14,10 @@ class MathController extends ChangeNotifier { set ast(SyntaxTree value) { if (_ast != value) { _ast = value; - _selection = const TextSelection.collapsed(offset: -1); + // Preserve the selection across tree mutations, clamped to the new + // tree's valid range. Required for editable-math widgets that need + // the caret to survive insertions/deletions. + _selection = sanitizeSelection(_ast, _selection); notifyListeners(); } } From 6352387329eaa4ba4bf1d5b72dfbe6ba7d406be1 Mon Sep 17 00:00:00 2001 From: Shubham Date: Mon, 13 Apr 2026 01:19:42 +0530 Subject: [PATCH 3/5] feat(widgets): accept an external MathController; fix caret showCursor plumbing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related changes required for downstream packages that want to build editable math (read: caret that survives tree mutations, driven by external input logic): 1. **External controller.** `SelectableMath` and `InternalSelectableMath` now accept an optional `controller: MathController`. When provided, the widget uses it instead of creating its own, and doesn't dispose it on unmount. `didUpdateWidget` handles both the owned-controller case (mutates `controller.ast`, preserving selection via the setter fix in the previous commit) and the external-controller case (passes through without re-creating). Also exports `MathController` from the public library surface so callers can actually construct one. 2. **showCursor in SelectionStyle.** `InternalSelectableMath.build` constructed the `SelectionStyle` Provider value without passing `widget.showCursor`. The render path (`EditableLine` → `RenderEditableLine.paint`) reads `showCursor` from that SelectionStyle, so the caret was never painted regardless of what the caller passed to `SelectableMath(showCursor: true)`. Now threaded through. --- lib/flutter_math.dart | 1 + lib/src/widgets/selectable.dart | 41 +++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/flutter_math.dart b/lib/flutter_math.dart index cc757a85..0fce1855 100644 --- a/lib/flutter_math.dart +++ b/lib/flutter_math.dart @@ -11,4 +11,5 @@ export 'src/parser/tex/parse_error.dart'; export 'src/parser/tex/settings.dart'; export 'src/widgets/exception.dart'; export 'src/widgets/math.dart'; +export 'src/widgets/controller.dart' show MathController; export 'src/widgets/selectable.dart' show SelectableMath; diff --git a/lib/src/widgets/selectable.dart b/lib/src/widgets/selectable.dart index a23f5e94..740383c4 100644 --- a/lib/src/widgets/selectable.dart +++ b/lib/src/widgets/selectable.dart @@ -41,6 +41,7 @@ class SelectableMath extends StatelessWidget { const SelectableMath({ Key? key, this.ast, + this.controller, this.autofocus = false, this.cursorColor, this.cursorRadius, @@ -72,6 +73,12 @@ class SelectableMath extends StatelessWidget { /// It can be null only when [parseException] is not null. final SyntaxTree? ast; + /// Optional externally-owned [MathController]. When non-null, the widget + /// uses this controller instead of creating its own. Lets callers drive + /// the selection (cursor position) from outside — required for editable + /// math widgets that mutate the tree at the cursor. + final MathController? controller; + /// {@macro flutter.widgets.editableText.autofocus} final bool autofocus; @@ -308,6 +315,7 @@ class SelectableMath extends StatelessWidget { return RepaintBoundary( child: InternalSelectableMath( ast: ast!, + controller: controller, autofocus: autofocus, cursorColor: cursorColor, cursorOffset: cursorOffset, @@ -339,6 +347,7 @@ class InternalSelectableMath extends StatefulWidget { const InternalSelectableMath({ Key? key, required this.ast, + this.controller, this.autofocus = false, required this.cursorColor, this.cursorOffset, @@ -361,6 +370,9 @@ class InternalSelectableMath extends StatefulWidget { final SyntaxTree ast; + /// Optional externally-owned [MathController]. See [SelectableMath.controller]. + final MathController? controller; + final bool autofocus; final Color cursorColor; @@ -424,20 +436,38 @@ class InternalSelectableMathState extends State DragStartBehavior get dragStartBehavior => widget.dragStartBehavior; late MathController controller; + bool _ownsController = true; late FocusNode _oldFocusNode; @override void initState() { - controller = MathController(ast: widget.ast); + if (widget.controller != null) { + controller = widget.controller!; + _ownsController = false; + } else { + controller = MathController(ast: widget.ast); + _ownsController = true; + } _oldFocusNode = focusNode..addListener(updateKeepAlive); super.initState(); } @override void didUpdateWidget(InternalSelectableMath oldWidget) { - if (widget.ast != controller.ast) { - controller = MathController(ast: widget.ast); + if (widget.controller != oldWidget.controller) { + if (widget.controller != null) { + controller = widget.controller!; + _ownsController = false; + } else { + controller = MathController(ast: widget.ast); + _ownsController = true; + } + } else if (_ownsController && widget.ast != controller.ast) { + // We own the controller — update the ast on the existing instance so + // the selection is preserved (via MathController.ast setter, which + // now sanitizes the selection against the new tree instead of resetting). + controller.ast = widget.ast; } if (_oldFocusNode != focusNode) { _oldFocusNode.removeListener(updateKeepAlive); @@ -465,7 +495,9 @@ class InternalSelectableMathState extends State void dispose() { _oldFocusNode.removeListener(updateKeepAlive); super.dispose(); - controller.dispose(); + if (_ownsController) { + controller.dispose(); + } } void onSelectionChanged( @@ -512,6 +544,7 @@ class InternalSelectableMathState extends State cursorHeight: widget.cursorHeight, selectionColor: widget.selectionColor, paintCursorAboveText: widget.paintCursorAboveText, + showCursor: widget.showCursor, ), ), Provider.value( From 5c5078faea0e8f012abae3371f4fc8c1a548ef40 Mon Sep 17 00:00:00 2001 From: Shubham Date: Mon, 13 Apr 2026 01:19:56 +0530 Subject: [PATCH 4/5] fix(render): anchor caret to alphabetic baseline; stable baseline for empty rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two caret-positioning bugs that only manifested for editable use: 1. **Caret drifted below text on lines with sup/sub/frac.** `RenderEditableLine.paint` anchored the caret at `Offset(cursorOffset, size.height)`. For any row containing a superscript, subscript, or fraction, `size.height` is substantially larger than the alphabetic baseline (it includes the ascent of the superscript / the height of the fraction). The caret ended up painted well below the main text line. Anchor at the row's actual alphabetic baseline (`maxHeightAboveBaseline`) instead. 2. **Empty rows reported zero baseline.** `computeDistanceToActualBaseline` on an empty row returned 0 (no children → `maxHeightAboveBaseline` is 0), which broke ancestor `Baseline` widgets: the row would align at Y=0 when empty, then jump when the first character was typed as the real baseline appeared. Override `computeDistanceToActualBaseline` on `RenderEditableLine` to fall back to `~80% of preferredLineHeight` when there are no children. Same fallback is used for the caret paint, so paint and layout always agree — empty↔non-empty transitions no longer shift the caret visually. --- lib/src/render/layout/line_editable.dart | 25 +++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/src/render/layout/line_editable.dart b/lib/src/render/layout/line_editable.dart index c9b1a3f4..4a593eec 100644 --- a/lib/src/render/layout/line_editable.dart +++ b/lib/src/render/layout/line_editable.dart @@ -308,6 +308,23 @@ class RenderEditableLine extends RenderLine { double preferredLineHeight; + /// Effective baseline used for cursor positioning and for reporting + /// this row's distance-to-baseline to ancestors (e.g. [Baseline] + /// widgets). Empty rows have no natural baseline (no children → + /// `maxHeightAboveBaseline` is 0), so we fall back to ~80% of + /// [preferredLineHeight] — roughly where a typical font's alphabetic + /// baseline sits. This keeps the caret stable across empty↔non-empty + /// transitions when the row is inside a [Baseline] widget. + double get _effectiveBaselineY => maxHeightAboveBaseline > 0 + ? maxHeightAboveBaseline + : preferredLineHeight * 0.8; + + @override + double computeDistanceToActualBaseline(TextBaseline baseline) { + assert(!debugNeedsLayout); + return _effectiveBaselineY; + } + TextSelection get selection => _selection; TextSelection _selection; set selection(TextSelection value) { @@ -447,7 +464,13 @@ class RenderEditableLine extends RenderLine { if (showCursor && _selection.isCollapsed && isSelectionInRange) { final cursorOffset = caretOffsets[selection.baseOffset]; - _paintCaret(context.canvas, Offset(cursorOffset, size.height) + offset); + // Anchor the caret at the alphabetic baseline (reported via + // [computeDistanceToActualBaseline]), not the bottom of the render + // box. For rows containing superscripts / fractions, size.height is + // much greater than the baseline, and using it pushes the caret + // visually below the main text. + _paintCaret( + context.canvas, Offset(cursorOffset, _effectiveBaselineY) + offset); } if (!_paintCursorAboveText) { From 89517677fe90b849cc83e3cfda9ffaf24fb55236 Mon Sep 17 00:00:00 2001 From: Shubham Date: Mon, 13 Apr 2026 01:55:27 +0530 Subject: [PATCH 5/5] fix: address CodeRabbit review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small fixes from the automated PR review: 1. `selectable.dart`: `InternalSelectableMathState.didUpdateWidget` was overwriting the `controller` field when `widget.controller` changed without first disposing the previously-owned instance. This leaked the internally-created MathController's listeners + notifiers whenever the caller swapped from null → external (or back). Now disposes the owned controller before the reassignment. 2. `syntax_tree.dart`: Removed a dead `newNode == null` check inside `_rebuildParent` — `newNode` is statically typed `GreenNode` (non-nullable), so the branch was unreachable. --- lib/src/ast/syntax_tree.dart | 3 ++- lib/src/widgets/selectable.dart | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/src/ast/syntax_tree.dart b/lib/src/ast/syntax_tree.dart index ddc6c822..b5b91496 100644 --- a/lib/src/ast/syntax_tree.dart +++ b/lib/src/ast/syntax_tree.dart @@ -89,8 +89,9 @@ class SyntaxTree { nullableChildren.add(null); hasNulls = true; } else if (identical(child, pos)) { + // newNode has static type GreenNode (non-null), so no need to set + // hasNulls here; the cast validates the element type. nullableChildren.add(newNode as EquationRowNode?); - if (newNode == null) hasNulls = true; } else { nullableChildren.add(child.value as EquationRowNode); } diff --git a/lib/src/widgets/selectable.dart b/lib/src/widgets/selectable.dart index 740383c4..9087391a 100644 --- a/lib/src/widgets/selectable.dart +++ b/lib/src/widgets/selectable.dart @@ -456,6 +456,12 @@ class InternalSelectableMathState extends State @override void didUpdateWidget(InternalSelectableMath oldWidget) { if (widget.controller != oldWidget.controller) { + // Dispose the previously-owned controller before swapping, otherwise + // the instance created in initState (or the last didUpdateWidget) + // leaks its listeners and internal state. + if (_ownsController) { + controller.dispose(); + } if (widget.controller != null) { controller = widget.controller!; _ownsController = false;