From 3df35979d29cbeda376e2e382d5b325b4d9fa50e Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 23 Feb 2021 10:45:40 -0800 Subject: [PATCH 1/2] [web] Fix placeholder-only paragraphs --- .../lib/src/engine/text/layout_service.dart | 33 ++++++++++++++----- .../test/text/layout_service_rich_test.dart | 18 ++++++++++ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/layout_service.dart b/lib/web_ui/lib/src/engine/text/layout_service.dart index 57cf525004f71..e00f8ffa27a28 100644 --- a/lib/web_ui/lib/src/engine/text/layout_service.dart +++ b/lib/web_ui/lib/src/engine/text/layout_service.dart @@ -171,9 +171,15 @@ class TextLayoutService { // ********************************************* // // Only go to the next span if we've reached the end of this span. - if (currentLine.end.index >= span.end && spanIndex < spanCount - 1) { + if (currentLine.end.index >= span.end) { currentLine.createBox(); - span = paragraph.spans[++spanIndex]; + if (spanIndex < spanCount - 1) { + span = paragraph.spans[++spanIndex]; + } else { + // We reached the end of the last span in the paragraph. + lines.add(currentLine.build()); + break; + } } } @@ -219,17 +225,23 @@ class TextLayoutService { minIntrinsicWidth = widthOfLastSegment; } + // Max intrinsic width includes the width of trailing spaces. + if (maxIntrinsicWidth < currentLine.widthIncludingSpace) { + maxIntrinsicWidth = currentLine.widthIncludingSpace; + } + if (currentLine.end.isHard) { - // Max intrinsic width includes the width of trailing spaces. - if (maxIntrinsicWidth < currentLine.widthIncludingSpace) { - maxIntrinsicWidth = currentLine.widthIncludingSpace; - } currentLine = currentLine.nextLine(); } // Only go to the next span if we've reached the end of this span. - if (currentLine.end.index >= span.end && spanIndex < spanCount - 1) { - span = paragraph.spans[++spanIndex]; + if (currentLine.end.index >= span.end) { + if (spanIndex < spanCount - 1) { + span = paragraph.spans[++spanIndex]; + } else { + // We reached the end of the last span in the paragraph. + break; + } } } } @@ -632,7 +644,10 @@ class LineSegment { double get widthOfTrailingSpace => widthIncludingSpace - width; /// Whether this segment is made of only white space. - bool get isSpaceOnly => start.index == end.indexWithoutTrailingSpaces; + /// + /// We rely on the [width] to determine this because relying on incides + /// doesn't work well for placeholders (they are zero-length strings). + bool get isSpaceOnly => width == 0; } /// Builds instances of [EngineLineMetrics] for the given [paragraph]. diff --git a/lib/web_ui/test/text/layout_service_rich_test.dart b/lib/web_ui/test/text/layout_service_rich_test.dart index d6b9df01e9bb9..5978658c21390 100644 --- a/lib/web_ui/test/text/layout_service_rich_test.dart +++ b/lib/web_ui/test/text/layout_service_rich_test.dart @@ -149,4 +149,22 @@ void testMain() async { l('ipsum', 6, 11, hardBreak: true, width: 50.0, left: 0.0), ]); }); + + test('should handle placeholder-only paragraphs', () { + final EngineParagraphStyle paragraphStyle = EngineParagraphStyle( + fontFamily: 'ahem', + fontSize: 10, + textAlign: ui.TextAlign.center, + ); + final CanvasParagraph paragraph = rich(paragraphStyle, (builder) { + builder.addPlaceholder(300.0, 50.0, ui.PlaceholderAlignment.baseline, baseline: ui.TextBaseline.alphabetic); + })..layout(constrain(500.0)); + + expect(paragraph.maxIntrinsicWidth, 300.0); + expect(paragraph.minIntrinsicWidth, 300.0); + expect(paragraph.height, 50.0); + expectLines(paragraph, [ + l('', 0, 0, hardBreak: false, width: 300.0, left: 100.0), + ]); + }); } From fa7328d6b6e174416f8c23df0d332e1e4414f4fd Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 23 Feb 2021 13:46:49 -0800 Subject: [PATCH 2/2] Fix failing tests --- .../lib/src/engine/text/layout_service.dart | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/layout_service.dart b/lib/web_ui/lib/src/engine/text/layout_service.dart index e00f8ffa27a28..5a47175da03e0 100644 --- a/lib/web_ui/lib/src/engine/text/layout_service.dart +++ b/lib/web_ui/lib/src/engine/text/layout_service.dart @@ -107,6 +107,8 @@ class TextLayoutService { // *** THE MAIN MEASUREMENT PART *** // // ********************************* // + final isLastSpan = spanIndex == spanCount - 1; + if (span is PlaceholderSpan) { if (currentLine.widthIncludingSpace + span.width <= constraints.width) { // The placeholder fits on the current line. @@ -119,6 +121,11 @@ class TextLayoutService { } currentLine.addPlaceholder(span); } + + if (isLastSpan) { + lines.add(currentLine.build()); + break; + } } else if (span is FlatTextSpan) { spanometer.currentSpan = span; final LineBreakResult nextBreak = currentLine.findNextBreak(span.end); @@ -171,15 +178,9 @@ class TextLayoutService { // ********************************************* // // Only go to the next span if we've reached the end of this span. - if (currentLine.end.index >= span.end) { + if (currentLine.end.index >= span.end && spanIndex < spanCount - 1) { currentLine.createBox(); - if (spanIndex < spanCount - 1) { - span = paragraph.spans[++spanIndex]; - } else { - // We reached the end of the last span in the paragraph. - lines.add(currentLine.build()); - break; - } + span = paragraph.spans[++spanIndex]; } }