From 7cf2dbdf3722339f9755f85703cdfe2822c3c076 Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Wed, 9 Mar 2016 00:38:21 -0800 Subject: [PATCH] Fix crash when dumping the app if it uses RichText Specifically: * Handle null styles in TextSpan without crashing in toString(). * Handle null children in TextSpan child lists without crashing in toString(). * Handle entirely empty TextSpans in toString() explicitly. * Assert that TextSpans don't contain nulls in various places. This is done more often than one might think necessary, because it turns out that TextSpan takes a (mutable) List for one of its arguments, so who knows what it will contain at any given time. By asserting all over the place, hopefully we'll catch it near the change if they do change it. * Add a RichText example to Stocks to exercise RichText and TextSpans. See also: https://github.com/flutter/flutter/issues/2514, https://github.com/flutter/flutter/issues/2519 --- examples/stocks/lib/stock_symbol_viewer.dart | 13 ++++++ packages/flutter/lib/services.dart | 1 + .../lib/src/painting/text_painter.dart | 45 +++++++++++++++++-- .../flutter/lib/src/rendering/paragraph.dart | 2 + .../flutter/lib/src/services/assertions.dart | 9 ++++ .../flutter/test/painting/text_span_test.dart | 34 ++++++++++++++ 6 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 packages/flutter/lib/src/services/assertions.dart diff --git a/examples/stocks/lib/stock_symbol_viewer.dart b/examples/stocks/lib/stock_symbol_viewer.dart index d718e385ec..308247da34 100644 --- a/examples/stocks/lib/stock_symbol_viewer.dart +++ b/examples/stocks/lib/stock_symbol_viewer.dart @@ -46,6 +46,19 @@ class StockSymbolView extends StatelessComponent { ), new Text('Market Cap', style: headings), new Text('${stock.marketCap}'), + new Container( + height: 8.0 + ), + new RichText( + text: new TextSpan( + style: DefaultTextStyle.of(context).merge(new TextStyle(fontSize: 8.0)), + text: 'Prices may be delayed by ', + children: [ + new TextSpan(text: 'several', style: new TextStyle(fontStyle: FontStyle.italic)), + new TextSpan(text: ' years.'), + ] + ) + ), ], justifyContent: FlexJustifyContent.collapse ) diff --git a/packages/flutter/lib/services.dart b/packages/flutter/lib/services.dart index 23d398f710..27e62eff90 100644 --- a/packages/flutter/lib/services.dart +++ b/packages/flutter/lib/services.dart @@ -12,6 +12,7 @@ library services; export 'src/services/activity.dart'; +export 'src/services/assertions.dart'; export 'src/services/asset_bundle.dart'; export 'src/services/binding.dart'; export 'src/services/fetch.dart'; diff --git a/packages/flutter/lib/src/painting/text_painter.dart b/packages/flutter/lib/src/painting/text_painter.dart index f53ea53cb8..ab8b69b83e 100644 --- a/packages/flutter/lib/src/painting/text_painter.dart +++ b/packages/flutter/lib/src/painting/text_painter.dart @@ -5,6 +5,7 @@ import 'dart:ui' as ui show Paragraph, ParagraphBuilder, ParagraphStyle, TextBox; import 'package:flutter/gestures.dart'; +import 'package:flutter/services.dart'; import 'basic_types.dart'; import 'text_editing.dart'; @@ -51,6 +52,7 @@ class TextSpan { final GestureRecognizer recognizer; void build(ui.ParagraphBuilder builder) { + assert(debugAssertValid()); final bool hasStyle = style != null; if (hasStyle) builder.pushStyle(style.textStyle); @@ -81,6 +83,7 @@ class TextSpan { } TextSpan getSpanForPosition(TextPosition position) { + assert(debugAssertValid()); TextAffinity affinity = position.affinity; int targetOffset = position.offset; int offset = 0; @@ -101,6 +104,7 @@ class TextSpan { } String toPlainText() { + assert(debugAssertValid()); StringBuffer buffer = new StringBuffer(); visitTextSpan((TextSpan span) { buffer.write(span.text); @@ -113,15 +117,47 @@ class TextSpan { StringBuffer buffer = new StringBuffer(); buffer.writeln('$prefix$runtimeType:'); String indent = '$prefix '; - buffer.writeln(style.toString(indent)); + if (style != null) + buffer.writeln(style.toString(indent)); if (text != null) buffer.writeln('$indent"$text"'); - if (children != null) - for (TextSpan child in children) - buffer.writeln(child.toString(indent)); + if (children != null) { + for (TextSpan child in children) { + if (child != null) { + buffer.write(child.toString(indent)); + } else { + buffer.writeln('$indent'); + } + } + } + if (style == null && text == null && children == null) + buffer.writeln('$indent(empty)'); return buffer.toString(); } + bool debugAssertValid() { + assert(() { + if (!visitTextSpan((TextSpan span) { + if (span.children != null) { + for (TextSpan child in span.children) { + if (child == null) + return false; + } + } + return true; + })) { + throw new FlutterError( + 'TextSpan contains a null child.\n' + 'A TextSpan object with a non-null child list should not have any nulls in its child list.\n' + 'The full text in question was:\n' + '${toString(" ")}' + ); + } + return true; + }); + return true; + } + bool operator ==(dynamic other) { if (identical(this, other)) return true; @@ -149,6 +185,7 @@ class TextPainter { /// The (potentially styled) text to paint. TextSpan get text => _text; void set text(TextSpan value) { + assert(value == null || value.debugAssertValid()); if (_text == value) return; _text = value; diff --git a/packages/flutter/lib/src/rendering/paragraph.dart b/packages/flutter/lib/src/rendering/paragraph.dart index f86a4ece29..b158b53370 100644 --- a/packages/flutter/lib/src/rendering/paragraph.dart +++ b/packages/flutter/lib/src/rendering/paragraph.dart @@ -15,6 +15,7 @@ class RenderParagraph extends RenderBox { TextSpan text ) : _textPainter = new TextPainter(text) { assert(text != null); + assert(text.debugAssertValid()); } final TextPainter _textPainter; @@ -24,6 +25,7 @@ class RenderParagraph extends RenderBox { /// The text to display TextSpan get text => _textPainter.text; void set text(TextSpan value) { + assert(value.debugAssertValid()); if (_textPainter.text == value) return; _textPainter.text = value; diff --git a/packages/flutter/lib/src/services/assertions.dart b/packages/flutter/lib/src/services/assertions.dart new file mode 100644 index 0000000000..26aaab3bff --- /dev/null +++ b/packages/flutter/lib/src/services/assertions.dart @@ -0,0 +1,9 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +class FlutterError extends AssertionError { + FlutterError(this.message); + final String message; + String toString() => message; +} diff --git a/packages/flutter/test/painting/text_span_test.dart b/packages/flutter/test/painting/text_span_test.dart index 9a358380ef..d43e797ebd 100644 --- a/packages/flutter/test/painting/text_span_test.dart +++ b/packages/flutter/test/painting/text_span_test.dart @@ -27,4 +27,38 @@ void main() { expect(b1 == a2, isFalse); expect(c1 == b2, isFalse); }); + + test("TextSpan ", () { + final TextSpan test = new TextSpan( + text: 'a', + style: new TextStyle( + fontSize: 10.0 + ), + children: [ + new TextSpan( + text: 'b', + children: [ + new TextSpan() + ] + ), + null, + new TextSpan( + text: 'c' + ), + ] + ); + expect(test.toString(), equals( + 'TextSpan:\n' + ' inherit: true\n' + ' size: 10.0\n' + ' "a"\n' + ' TextSpan:\n' + ' "b"\n' + ' TextSpan:\n' + ' (empty)\n' + ' \n' + ' TextSpan:\n' + ' "c"\n' + )); + }); }