From 3173620eb86eecae2ddd489ac2a186cf227e962d Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 6 Mar 2020 13:05:37 -0800 Subject: [PATCH 1/6] Work around line measurement rounding bug in Firefox. Add -webkit prefix for Safari text decoration --- lib/web_ui/lib/src/engine/text/paragraph.dart | 49 +++++++++++-------- lib/web_ui/lib/src/engine/text/ruler.dart | 27 ++++++++-- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 0b761d64e962d..83ec07b875f25 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -396,11 +396,13 @@ class EngineParagraph implements ui.Paragraph { ui.TextBox _getBoxForLine(EngineLineMetrics line, int start, int end) { final double widthBeforeBox = start <= line.startIndex - ? 0.0 - : _measurementService.measureSubstringWidth(this, line.startIndex, start); + ? 0.0 + : _measurementService.measureSubstringWidth( + this, line.startIndex, start); final double widthAfterBox = end >= line.endIndexWithoutNewlines - ? 0.0 - : _measurementService.measureSubstringWidth(this, end, line.endIndexWithoutNewlines); + ? 0.0 + : _measurementService.measureSubstringWidth( + this, end, line.endIndexWithoutNewlines); final double top = line.lineNumber * _lineHeight; @@ -488,7 +490,8 @@ class EngineParagraph implements ui.Paragraph { int high = lineMetrics.endIndexWithoutNewlines; do { final int current = (low + high) ~/ 2; - final double width = instance.measureSubstringWidth(this, lineMetrics.startIndex, current); + final double width = + instance.measureSubstringWidth(this, lineMetrics.startIndex, current); if (width < dx) { low = current; } else if (width > dx) { @@ -503,8 +506,10 @@ class EngineParagraph implements ui.Paragraph { return ui.TextPosition(offset: high, affinity: ui.TextAffinity.upstream); } - final double lowWidth = instance.measureSubstringWidth(this, lineMetrics.startIndex, low); - final double highWidth = instance.measureSubstringWidth(this, lineMetrics.startIndex, high); + final double lowWidth = + instance.measureSubstringWidth(this, lineMetrics.startIndex, low); + final double highWidth = + instance.measureSubstringWidth(this, lineMetrics.startIndex, high); if (dx - lowWidth < highWidth - dx) { // The offset is closer to the low index. @@ -666,18 +671,17 @@ class EngineParagraphStyle implements ui.ParagraphStyle { @override int get hashCode { return ui.hashValues( - _textAlign, - _textDirection, - _fontWeight, - _fontStyle, - _maxLines, - _fontFamily, - _fontSize, - _height, - _textHeightBehavior, - _ellipsis, - _locale - ); + _textAlign, + _textDirection, + _fontWeight, + _fontStyle, + _maxLines, + _fontFamily, + _fontSize, + _height, + _textHeightBehavior, + _ellipsis, + _locale); } @override @@ -1500,7 +1504,12 @@ void _applyTextStyleToElement({ final String textDecoration = _textDecorationToCssString(style._decoration, style._decorationStyle); if (textDecoration != null) { - cssStyle.textDecoration = textDecoration; + if (browserEngine == BrowserEngine.webkit) { + domRenderer.setElementStyle( + element, '-webkit-text-decoration', textDecoration); + } else { + cssStyle.textDecoration = textDecoration; + } final ui.Color decorationColor = style._decorationColor; if (decorationColor != null) { cssStyle.textDecorationColor = colorToCssString(decorationColor); diff --git a/lib/web_ui/lib/src/engine/text/ruler.dart b/lib/web_ui/lib/src/engine/text/ruler.dart index 08ac9c89a0cfd..e83466b206a79 100644 --- a/lib/web_ui/lib/src/engine/text/ruler.dart +++ b/lib/web_ui/lib/src/engine/text/ruler.dart @@ -244,7 +244,8 @@ class TextDimensions { /// Applies geometric style properties to the [element]. void applyStyle(ParagraphGeometricStyle style) { - _element.style + final html.CssStyleDeclaration elementStyle = _element.style; + elementStyle ..fontSize = style.fontSize != null ? '${style.fontSize.floor()}px' : null ..fontFamily = canonicalizeFontFamily(style.effectiveFontFamily) ..fontWeight = @@ -255,10 +256,16 @@ class TextDimensions { ..letterSpacing = style.letterSpacing != null ? '${style.letterSpacing}px' : null ..wordSpacing = - style.wordSpacing != null ? '${style.wordSpacing}px' : null - ..textDecoration = style.decoration; + style.wordSpacing != null ? '${style.wordSpacing}px' : null; + final String decoration = style.decoration; + if (browserEngine == BrowserEngine.webkit) { + domRenderer.setElementStyle( + _element, '-webkit-text-decoration', decoration); + } else { + elementStyle.textDecoration = decoration; + } if (style.lineHeight != null) { - _element.style.lineHeight = style.lineHeight.toString(); + elementStyle.lineHeight = style.lineHeight.toString(); } _invalidateBoundsCache(); } @@ -277,7 +284,17 @@ class TextDimensions { double get width => _readAndCacheMetrics().width; /// The height of the paragraph being measured. - double get height => _readAndCacheMetrics().height; + double get height { + double cachedHeight = _readAndCacheMetrics().height; + if (browserEngine == BrowserEngine.firefox) { + // See subpixel rounding bug : + // https://bugzilla.mozilla.org/show_bug.cgi?id=442139 + // This causes bottom of letters such as 'y' to be cutoff and + // incorrect rendering of double underlines. + cachedHeight += 1.0; + } + return cachedHeight; + } } /// Performs 4 types of measurements: From e1620bfc6cddda93552fc9f980f35a89f963809a Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 9 Mar 2020 13:39:19 -0700 Subject: [PATCH 2/6] Update measurement test for firefox changes --- lib/web_ui/test/text/measurement_test.dart | 74 ++++++++++++---------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/lib/web_ui/test/text/measurement_test.dart b/lib/web_ui/test/text/measurement_test.dart index 919e5fc97ae61..5685d88f9f07b 100644 --- a/lib/web_ui/test/text/measurement_test.dart +++ b/lib/web_ui/test/text/measurement_test.dart @@ -160,7 +160,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 60); expect(result.minIntrinsicWidth, 30); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); expect(result.lines, [ line(' abc', 0, 6, hardBreak: true, width: 60.0, lineNumber: 0, left: 0.0), ]); @@ -170,7 +170,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 60); expect(result.minIntrinsicWidth, 30); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line('abc ', 0, 6, hardBreak: true, width: 30.0, lineNumber: 0, left: 0.0), @@ -188,7 +188,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 100); expect(result.minIntrinsicWidth, 20); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line(' ab c ', 0, 10, hardBreak: true, width: 80.0, lineNumber: 0, left: 0.0), @@ -206,7 +206,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 10); expect(result.minIntrinsicWidth, 0); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line(' ', 0, 1, hardBreak: true, width: 0.0, lineNumber: 0, left: 0.0), @@ -224,7 +224,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 50); expect(result.minIntrinsicWidth, 0); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line(' ', 0, 5, hardBreak: true, width: 0.0, lineNumber: 0, left: 0.0), @@ -250,7 +250,7 @@ void main() async { expect(result.maxIntrinsicWidth, 50); expect(result.minIntrinsicWidth, 50); expect(result.width, 50); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); expect(result.lines, [ line('12345', 0, 5, hardBreak: true, width: 50.0, lineNumber: 0, left: 0.0), ]); @@ -270,7 +270,7 @@ void main() async { expect(result.maxIntrinsicWidth, 110); expect(result.minIntrinsicWidth, 30); expect(result.width, 70); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('foo bar ', 0, 8, hardBreak: false, width: 70.0, lineNumber: 0, left: 0.0), @@ -295,7 +295,7 @@ void main() async { expect(result.maxIntrinsicWidth, 100); expect(result.minIntrinsicWidth, 100); expect(result.width, 50); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('12345', 0, 5, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -314,7 +314,7 @@ void main() async { expect(result.maxIntrinsicWidth, 140); expect(result.minIntrinsicWidth, 110); expect(result.width, 50); - expect(result.height, 30); + expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('abcde', 0, 5, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -336,7 +336,7 @@ void main() async { expect(result.maxIntrinsicWidth, 20); expect(result.minIntrinsicWidth, 20); expect(result.width, 8); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('A', 0, 1, hardBreak: false, width: 10.0, lineNumber: 0, left: 0.0), @@ -354,7 +354,7 @@ void main() async { expect(result.maxIntrinsicWidth, 20); expect(result.minIntrinsicWidth, 20); expect(result.width, 8); - expect(result.height, 30); + expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('A', 0, 1, hardBreak: false, width: 10.0, lineNumber: 0, left: 0.0), @@ -373,7 +373,7 @@ void main() async { expect(result.maxIntrinsicWidth, 30); expect(result.minIntrinsicWidth, 30); expect(result.width, 8); - expect(result.height, 40); + expect(result.height, normalizedHeight(40, instance.isCanvas ? 4 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('A', 0, 1, hardBreak: false, width: 10.0, lineNumber: 0, left: 0.0), @@ -401,7 +401,7 @@ void main() async { expect(result.maxIntrinsicWidth, 20); expect(result.minIntrinsicWidth, 20); expect(result.width, 50); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('12', 0, 3, hardBreak: true, width: 20.0, lineNumber: 0, left: 0.0), @@ -422,7 +422,7 @@ void main() async { result = instance.measure(build(ahemStyle, '\n\n1234'), constraints); expect(result.maxIntrinsicWidth, 40); expect(result.minIntrinsicWidth, 40); - expect(result.height, 30); + expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('', 0, 1, hardBreak: true, width: 0.0, lineNumber: 0, left: 0.0), @@ -439,7 +439,7 @@ void main() async { result = instance.measure(build(ahemStyle, '12\n\n345'), constraints); expect(result.maxIntrinsicWidth, 30); expect(result.minIntrinsicWidth, 30); - expect(result.height, 30); + expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('12', 0, 3, hardBreak: true, width: 20.0, lineNumber: 0, left: 0.0), @@ -458,7 +458,7 @@ void main() async { expect(result.minIntrinsicWidth, 40); if (instance.isCanvas) { // This can only be done correctly in the canvas-based implementation. - expect(result.height, 30); + expect(result.height, normalizedHeight(30, 3)); expect(result.lines, [ line('1234', 0, 5, hardBreak: true, width: 40.0, lineNumber: 0, left: 0.0), line('', 5, 6, hardBreak: true, width: 0.0, lineNumber: 1, left: 0.0), @@ -482,7 +482,7 @@ void main() async { expect(result.maxIntrinsicWidth, 70); expect(result.minIntrinsicWidth, 30); expect(result.width, double.infinity); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); if (instance.isCanvas) { expect(result.lines, [ @@ -731,7 +731,7 @@ void main() async { result = instance.measure(longText, constraints); expect(result.minIntrinsicWidth, 480); expect(result.maxIntrinsicWidth, 480); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line('AA...', 0, 48, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -751,7 +751,7 @@ void main() async { result = instance.measure(longTextShortPrefix, constraints); expect(result.minIntrinsicWidth, 450); expect(result.maxIntrinsicWidth, 450); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); if (instance.isCanvas) { expect(result.lines, [ line('AAA', 0, 4, hardBreak: true, width: 30.0, lineNumber: 0, left: 0.0), @@ -770,7 +770,7 @@ void main() async { result = instance.measure(text, tinyConstraints); expect(result.minIntrinsicWidth, 40); expect(result.maxIntrinsicWidth, 40); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line('...', 0, 4, hardBreak: false, width: 30.0, lineNumber: 0, left: 0.0), @@ -787,7 +787,7 @@ void main() async { result = instance.measure(text, tinierConstraints); expect(result.minIntrinsicWidth, 40); expect(result.maxIntrinsicWidth, 40); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { // TODO(flutter_web): https://github.com/flutter/flutter/issues/34346 // expect(result.lines, [ @@ -817,7 +817,7 @@ void main() async { // The height should be that of a single line. final ui.Paragraph oneline = build(maxlinesStyle, 'One line'); result = instance.measure(oneline, infiniteConstraints); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); expect(result.lines, [ line('One line', 0, 8, hardBreak: true, width: 80.0, lineNumber: 0, left: 0.0), ]); @@ -826,7 +826,7 @@ void main() async { final ui.Paragraph threelines = build(maxlinesStyle, 'First\nSecond\nThird'); result = instance.measure(threelines, infiniteConstraints); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); if (instance.isCanvas) { expect(result.lines, [ line('First', 0, 6, hardBreak: true, width: 50.0, lineNumber: 0, left: 0.0), @@ -844,7 +844,7 @@ void main() async { 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.', ); result = instance.measure(veryLong, constraints); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); if (instance.isCanvas) { expect(result.lines, [ line('Lorem ', 0, 6, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -862,7 +862,7 @@ void main() async { 'AAA AAAAAAAAAAAAAAAAAAA', ); result = instance.measure(veryLongLastLine, constraints); - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); if (instance.isCanvas) { expect(result.lines, [ line('AAA ', 0, 4, hardBreak: false, width: 30.0, lineNumber: 0, left: 0.0), @@ -899,7 +899,7 @@ void main() async { // Simple no overflow case. p = build(onelineStyle, 'abcdef'); result = instance.measure(p, constraints); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); expect(result.lines, [ line('abcdef', 0, 6, hardBreak: true, width: 60.0, lineNumber: 0, left: 0.0), ]); @@ -907,7 +907,7 @@ void main() async { // Simple overflow case. p = build(onelineStyle, 'abcd efg'); result = instance.measure(p, constraints); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line('abc...', 0, 8, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -921,7 +921,7 @@ void main() async { // Another simple overflow case. p = build(onelineStyle, 'a bcde fgh'); result = instance.measure(p, constraints); - expect(result.height, 10); + expect(result.height, normalizedHeight(10, 1)); if (instance.isCanvas) { expect(result.lines, [ line('a b...', 0, 10, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -938,7 +938,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); expect(result.lines, [ line('abcdef ', 0, 7, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -955,7 +955,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); expect(result.lines, [ line('abcd ', 0, 5, hardBreak: false, width: 40.0, lineNumber: 0, left: 0.0), @@ -973,7 +973,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); expect(result.lines, [ line('abcde ', 0, 6, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -990,7 +990,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); expect(result.lines, [ line('abcdef', 0, 6, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -1007,7 +1007,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, 20); + expect(result.height, normalizedHeight(20, 2)); expect(result.lines, [ line('abcdef', 0, 6, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -1156,3 +1156,11 @@ EngineLineMetrics line( left: left, ); } + +// Normalizes height across firefox and other browsers. +// +// Firefox reports incorrect height by rounding down bounds, therefore +// the ruler adds 1px by default to line heights to prevent +// underline/double underline, descender clipping. +double normalizedHeight(double value, int firefoxDelta) => + browserEngine == BrowserEngine.firefox ? value + firefoxDelta : value; From a7f16737088ec8f8772a5ca6d5bf14138faa27a9 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 9 Mar 2020 14:11:47 -0700 Subject: [PATCH 3/6] Update text_test to handle firefox --- lib/web_ui/test/text_test.dart | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/test/text_test.dart b/lib/web_ui/test/text_test.dart index 0c9a5c6c4ca0c..cf70e6035ab90 100644 --- a/lib/web_ui/test/text_test.dart +++ b/lib/web_ui/test/text_test.dart @@ -29,7 +29,7 @@ void main() async { final Paragraph paragraph = builder.build(); paragraph.layout(const ParagraphConstraints(width: 400.0)); - expect(paragraph.height, fontSize); + expect(paragraph.height, normalizedHeight(fontSize, 1)); expect(paragraph.width, 400.0); expect(paragraph.minIntrinsicWidth, fontSize * 4.0); expect(paragraph.maxIntrinsicWidth, fontSize * 4.0); @@ -55,7 +55,7 @@ void main() async { final Paragraph paragraph = builder.build(); paragraph.layout(ParagraphConstraints(width: fontSize * 5.0)); - expect(paragraph.height, fontSize * 2.0); // because it wraps + expect(paragraph.height, normalizedHeight(fontSize * 2.0, 1)); // because it wraps expect(paragraph.width, fontSize * 5.0); expect(paragraph.minIntrinsicWidth, fontSize * 4.0); @@ -493,3 +493,11 @@ void main() async { }); }); } + +// Normalizes height across firefox and other browsers. +// +// Firefox reports incorrect height by rounding down bounds, therefore +// the ruler adds 1px by default to line heights to prevent +// underline/double underline, descender clipping. +double normalizedHeight(double value, int firefoxDelta) => + browserEngine == BrowserEngine.firefox ? value + firefoxDelta : value; From 5c029b7fd794f41149f7362212e502fc6b064114 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 9 Mar 2020 18:40:47 -0700 Subject: [PATCH 4/6] Add check for measurement tests for firefox --- lib/web_ui/lib/src/engine/text/ruler.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/text/ruler.dart b/lib/web_ui/lib/src/engine/text/ruler.dart index e83466b206a79..b2e317d27f5ea 100644 --- a/lib/web_ui/lib/src/engine/text/ruler.dart +++ b/lib/web_ui/lib/src/engine/text/ruler.dart @@ -286,7 +286,10 @@ class TextDimensions { /// The height of the paragraph being measured. double get height { double cachedHeight = _readAndCacheMetrics().height; - if (browserEngine == BrowserEngine.firefox) { + if (browserEngine == BrowserEngine.firefox && + // In the flutter tester environment, we use a predictable-size for font + // measurement tests. + !ui.debugEmulateFlutterTesterEnvironment) { // See subpixel rounding bug : // https://bugzilla.mozilla.org/show_bug.cgi?id=442139 // This causes bottom of letters such as 'y' to be cutoff and From b729bc4d80b8fff0d7b70d6d3abae54356574362 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 9 Mar 2020 18:45:58 -0700 Subject: [PATCH 5/6] undo measurement_test changes for firefox --- lib/web_ui/test/text/measurement_test.dart | 74 ++++++++++------------ 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/lib/web_ui/test/text/measurement_test.dart b/lib/web_ui/test/text/measurement_test.dart index 5685d88f9f07b..919e5fc97ae61 100644 --- a/lib/web_ui/test/text/measurement_test.dart +++ b/lib/web_ui/test/text/measurement_test.dart @@ -160,7 +160,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 60); expect(result.minIntrinsicWidth, 30); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); expect(result.lines, [ line(' abc', 0, 6, hardBreak: true, width: 60.0, lineNumber: 0, left: 0.0), ]); @@ -170,7 +170,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 60); expect(result.minIntrinsicWidth, 30); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line('abc ', 0, 6, hardBreak: true, width: 30.0, lineNumber: 0, left: 0.0), @@ -188,7 +188,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 100); expect(result.minIntrinsicWidth, 20); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line(' ab c ', 0, 10, hardBreak: true, width: 80.0, lineNumber: 0, left: 0.0), @@ -206,7 +206,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 10); expect(result.minIntrinsicWidth, 0); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line(' ', 0, 1, hardBreak: true, width: 0.0, lineNumber: 0, left: 0.0), @@ -224,7 +224,7 @@ void main() async { result = instance.measure(text, infiniteConstraints); expect(result.maxIntrinsicWidth, 50); expect(result.minIntrinsicWidth, 0); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line(' ', 0, 5, hardBreak: true, width: 0.0, lineNumber: 0, left: 0.0), @@ -250,7 +250,7 @@ void main() async { expect(result.maxIntrinsicWidth, 50); expect(result.minIntrinsicWidth, 50); expect(result.width, 50); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); expect(result.lines, [ line('12345', 0, 5, hardBreak: true, width: 50.0, lineNumber: 0, left: 0.0), ]); @@ -270,7 +270,7 @@ void main() async { expect(result.maxIntrinsicWidth, 110); expect(result.minIntrinsicWidth, 30); expect(result.width, 70); - expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('foo bar ', 0, 8, hardBreak: false, width: 70.0, lineNumber: 0, left: 0.0), @@ -295,7 +295,7 @@ void main() async { expect(result.maxIntrinsicWidth, 100); expect(result.minIntrinsicWidth, 100); expect(result.width, 50); - expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('12345', 0, 5, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -314,7 +314,7 @@ void main() async { expect(result.maxIntrinsicWidth, 140); expect(result.minIntrinsicWidth, 110); expect(result.width, 50); - expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); + expect(result.height, 30); if (instance.isCanvas) { expect(result.lines, [ line('abcde', 0, 5, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -336,7 +336,7 @@ void main() async { expect(result.maxIntrinsicWidth, 20); expect(result.minIntrinsicWidth, 20); expect(result.width, 8); - expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('A', 0, 1, hardBreak: false, width: 10.0, lineNumber: 0, left: 0.0), @@ -354,7 +354,7 @@ void main() async { expect(result.maxIntrinsicWidth, 20); expect(result.minIntrinsicWidth, 20); expect(result.width, 8); - expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); + expect(result.height, 30); if (instance.isCanvas) { expect(result.lines, [ line('A', 0, 1, hardBreak: false, width: 10.0, lineNumber: 0, left: 0.0), @@ -373,7 +373,7 @@ void main() async { expect(result.maxIntrinsicWidth, 30); expect(result.minIntrinsicWidth, 30); expect(result.width, 8); - expect(result.height, normalizedHeight(40, instance.isCanvas ? 4 : 1)); + expect(result.height, 40); if (instance.isCanvas) { expect(result.lines, [ line('A', 0, 1, hardBreak: false, width: 10.0, lineNumber: 0, left: 0.0), @@ -401,7 +401,7 @@ void main() async { expect(result.maxIntrinsicWidth, 20); expect(result.minIntrinsicWidth, 20); expect(result.width, 50); - expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('12', 0, 3, hardBreak: true, width: 20.0, lineNumber: 0, left: 0.0), @@ -422,7 +422,7 @@ void main() async { result = instance.measure(build(ahemStyle, '\n\n1234'), constraints); expect(result.maxIntrinsicWidth, 40); expect(result.minIntrinsicWidth, 40); - expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); + expect(result.height, 30); if (instance.isCanvas) { expect(result.lines, [ line('', 0, 1, hardBreak: true, width: 0.0, lineNumber: 0, left: 0.0), @@ -439,7 +439,7 @@ void main() async { result = instance.measure(build(ahemStyle, '12\n\n345'), constraints); expect(result.maxIntrinsicWidth, 30); expect(result.minIntrinsicWidth, 30); - expect(result.height, normalizedHeight(30, instance.isCanvas ? 3 : 1)); + expect(result.height, 30); if (instance.isCanvas) { expect(result.lines, [ line('12', 0, 3, hardBreak: true, width: 20.0, lineNumber: 0, left: 0.0), @@ -458,7 +458,7 @@ void main() async { expect(result.minIntrinsicWidth, 40); if (instance.isCanvas) { // This can only be done correctly in the canvas-based implementation. - expect(result.height, normalizedHeight(30, 3)); + expect(result.height, 30); expect(result.lines, [ line('1234', 0, 5, hardBreak: true, width: 40.0, lineNumber: 0, left: 0.0), line('', 5, 6, hardBreak: true, width: 0.0, lineNumber: 1, left: 0.0), @@ -482,7 +482,7 @@ void main() async { expect(result.maxIntrinsicWidth, 70); expect(result.minIntrinsicWidth, 30); expect(result.width, double.infinity); - expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ @@ -731,7 +731,7 @@ void main() async { result = instance.measure(longText, constraints); expect(result.minIntrinsicWidth, 480); expect(result.maxIntrinsicWidth, 480); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line('AA...', 0, 48, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -751,7 +751,7 @@ void main() async { result = instance.measure(longTextShortPrefix, constraints); expect(result.minIntrinsicWidth, 450); expect(result.maxIntrinsicWidth, 450); - expect(result.height, normalizedHeight(20, instance.isCanvas ? 2 : 1)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('AAA', 0, 4, hardBreak: true, width: 30.0, lineNumber: 0, left: 0.0), @@ -770,7 +770,7 @@ void main() async { result = instance.measure(text, tinyConstraints); expect(result.minIntrinsicWidth, 40); expect(result.maxIntrinsicWidth, 40); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line('...', 0, 4, hardBreak: false, width: 30.0, lineNumber: 0, left: 0.0), @@ -787,7 +787,7 @@ void main() async { result = instance.measure(text, tinierConstraints); expect(result.minIntrinsicWidth, 40); expect(result.maxIntrinsicWidth, 40); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { // TODO(flutter_web): https://github.com/flutter/flutter/issues/34346 // expect(result.lines, [ @@ -817,7 +817,7 @@ void main() async { // The height should be that of a single line. final ui.Paragraph oneline = build(maxlinesStyle, 'One line'); result = instance.measure(oneline, infiniteConstraints); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); expect(result.lines, [ line('One line', 0, 8, hardBreak: true, width: 80.0, lineNumber: 0, left: 0.0), ]); @@ -826,7 +826,7 @@ void main() async { final ui.Paragraph threelines = build(maxlinesStyle, 'First\nSecond\nThird'); result = instance.measure(threelines, infiniteConstraints); - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('First', 0, 6, hardBreak: true, width: 50.0, lineNumber: 0, left: 0.0), @@ -844,7 +844,7 @@ void main() async { 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.', ); result = instance.measure(veryLong, constraints); - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('Lorem ', 0, 6, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -862,7 +862,7 @@ void main() async { 'AAA AAAAAAAAAAAAAAAAAAA', ); result = instance.measure(veryLongLastLine, constraints); - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); if (instance.isCanvas) { expect(result.lines, [ line('AAA ', 0, 4, hardBreak: false, width: 30.0, lineNumber: 0, left: 0.0), @@ -899,7 +899,7 @@ void main() async { // Simple no overflow case. p = build(onelineStyle, 'abcdef'); result = instance.measure(p, constraints); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); expect(result.lines, [ line('abcdef', 0, 6, hardBreak: true, width: 60.0, lineNumber: 0, left: 0.0), ]); @@ -907,7 +907,7 @@ void main() async { // Simple overflow case. p = build(onelineStyle, 'abcd efg'); result = instance.measure(p, constraints); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line('abc...', 0, 8, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -921,7 +921,7 @@ void main() async { // Another simple overflow case. p = build(onelineStyle, 'a bcde fgh'); result = instance.measure(p, constraints); - expect(result.height, normalizedHeight(10, 1)); + expect(result.height, 10); if (instance.isCanvas) { expect(result.lines, [ line('a b...', 0, 10, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -938,7 +938,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); expect(result.lines, [ line('abcdef ', 0, 7, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -955,7 +955,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); expect(result.lines, [ line('abcd ', 0, 5, hardBreak: false, width: 40.0, lineNumber: 0, left: 0.0), @@ -973,7 +973,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); expect(result.lines, [ line('abcde ', 0, 6, hardBreak: false, width: 50.0, lineNumber: 0, left: 0.0), @@ -990,7 +990,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); expect(result.lines, [ line('abcdef', 0, 6, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -1007,7 +1007,7 @@ void main() async { result = instance.measure(p, constraints); // This can only be done correctly in the canvas-based implementation. if (instance.isCanvas) { - expect(result.height, normalizedHeight(20, 2)); + expect(result.height, 20); expect(result.lines, [ line('abcdef', 0, 6, hardBreak: false, width: 60.0, lineNumber: 0, left: 0.0), @@ -1156,11 +1156,3 @@ EngineLineMetrics line( left: left, ); } - -// Normalizes height across firefox and other browsers. -// -// Firefox reports incorrect height by rounding down bounds, therefore -// the ruler adds 1px by default to line heights to prevent -// underline/double underline, descender clipping. -double normalizedHeight(double value, int firefoxDelta) => - browserEngine == BrowserEngine.firefox ? value + firefoxDelta : value; From b44fa2a4aa84f506fe10ce1cbc7a94ff1f3772f5 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 9 Mar 2020 18:47:59 -0700 Subject: [PATCH 6/6] undo text_test for firefox --- lib/web_ui/test/text_test.dart | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/web_ui/test/text_test.dart b/lib/web_ui/test/text_test.dart index cf70e6035ab90..0c9a5c6c4ca0c 100644 --- a/lib/web_ui/test/text_test.dart +++ b/lib/web_ui/test/text_test.dart @@ -29,7 +29,7 @@ void main() async { final Paragraph paragraph = builder.build(); paragraph.layout(const ParagraphConstraints(width: 400.0)); - expect(paragraph.height, normalizedHeight(fontSize, 1)); + expect(paragraph.height, fontSize); expect(paragraph.width, 400.0); expect(paragraph.minIntrinsicWidth, fontSize * 4.0); expect(paragraph.maxIntrinsicWidth, fontSize * 4.0); @@ -55,7 +55,7 @@ void main() async { final Paragraph paragraph = builder.build(); paragraph.layout(ParagraphConstraints(width: fontSize * 5.0)); - expect(paragraph.height, normalizedHeight(fontSize * 2.0, 1)); // because it wraps + expect(paragraph.height, fontSize * 2.0); // because it wraps expect(paragraph.width, fontSize * 5.0); expect(paragraph.minIntrinsicWidth, fontSize * 4.0); @@ -493,11 +493,3 @@ void main() async { }); }); } - -// Normalizes height across firefox and other browsers. -// -// Firefox reports incorrect height by rounding down bounds, therefore -// the ruler adds 1px by default to line heights to prevent -// underline/double underline, descender clipping. -double normalizedHeight(double value, int firefoxDelta) => - browserEngine == BrowserEngine.firefox ? value + firefoxDelta : value;