Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web] Fixes for Firefox & Safari double underline decoration bugs. #16994

Merged
merged 8 commits into from
Mar 10, 2020
49 changes: 29 additions & 20 deletions lib/web_ui/lib/src/engine/text/paragraph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
27 changes: 22 additions & 5 deletions lib/web_ui/lib/src/engine/text/ruler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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();
}
Expand All @@ -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:
Expand Down