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

Get rid of "outrageous" default text styles for HTML renderer. #41822

Merged
merged 6 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions lib/web_ui/lib/src/engine/global_styles.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,9 @@ void applyGlobalCssRulesToSheet(
assert(styleElement.sheet != null);
final DomCSSStyleSheet sheet = styleElement.sheet! as DomCSSStyleSheet;

// These are intentionally outrageous font parameters to make sure that the
// apps fully specify their text styles.
//
// Fixes #115216 by ensuring that our parameters only affect the flt-scene-host children.
sheet.insertRule('''
$cssSelectorPrefix flt-scene-host {
color: red;
font: $defaultCssFont;
}
''', sheet.cssRules.length);
Expand Down
4 changes: 1 addition & 3 deletions lib/web_ui/lib/src/engine/text/canvas_paragraph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import 'paint_service.dart';
import 'paragraph.dart';
import 'word_breaker.dart';

const ui.Color _defaultTextColor = ui.Color(0xFFFF0000);

final String placeholderChar = String.fromCharCode(0xFFFC);

/// A paragraph made up of a flat list of text spans and placeholders.
Expand Down Expand Up @@ -491,7 +489,7 @@ class RootStyleNode extends StyleNode {
final EngineParagraphStyle paragraphStyle;

@override
final ui.Color _color = _defaultTextColor;
ui.Color? get _color => null;

@override
ui.TextDecoration? get _decoration => null;
Expand Down
5 changes: 4 additions & 1 deletion lib/web_ui/lib/src/engine/text/paint_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ class TextPaintService {
if (foreground != null) {
paint = foreground as SurfacePaint;
} else {
paint = (ui.Paint()..color = style.color!) as SurfacePaint;
paint = ui.Paint() as SurfacePaint;
if (style.color != null) {
paint.color = style.color!;
}
}

canvas.setCssFont(style.cssFontString, fragment.textDirection!);
Expand Down
22 changes: 0 additions & 22 deletions lib/web_ui/test/engine/global_styles_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,6 @@ void testMain() {
expect(hasFakeRule, isFalse);
});

test('Attaches outrageous text styles to flt-scene-host', () {
final bool hasColorRed = hasCssRule(styleElement,
selector: 'flt-scene-host', declaration: 'color: red');

bool hasFont = false;
if (isSafari) {
// Safari expands the shorthand rules, so we check for all we've set (separately).
hasFont = hasCssRule(styleElement,
selector: 'flt-scene-host',
declaration: 'font-family: monospace') &&
hasCssRule(styleElement,
selector: 'flt-scene-host', declaration: 'font-size: 14px');
} else {
hasFont = hasCssRule(styleElement,
selector: 'flt-scene-host', declaration: 'font: $_kDefaultCssFont');
}

expect(hasColorRed, isTrue,
reason: 'Should make foreground color red within scene host.');
expect(hasFont, isTrue, reason: 'Should pass default css font.');
});

test('Attaches styling to remove password reveal icons on Edge', () {
// Check that style.sheet! contains input::-ms-reveal rule
final bool hidesRevealIcons = hasCssRule(styleElement,
Expand Down
3 changes: 0 additions & 3 deletions lib/web_ui/test/html/text/canvas_paragraph_builder_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,6 @@ String spanStyle({
num? letterSpacing,
}) {
return <String>[
'color: rgb(255, 0, 0);',
'font-size: ${fontSize}px;',
if (fontWeight != null) 'font-weight: $fontWeight;',
if (fontStyle != null) 'font-style: $fontStyle;',
Expand All @@ -529,7 +528,6 @@ String spanStyle({
}

TextStyle styleWithDefaults({
Color color = const Color(0xFFFF0000),
String fontFamily = FlutterViewEmbedder.defaultFontFamily,
double fontSize = FlutterViewEmbedder.defaultFontSize,
FontWeight? fontWeight,
Expand All @@ -538,7 +536,6 @@ TextStyle styleWithDefaults({
double? letterSpacing,
}) {
return TextStyle(
color: color,
fontFamily: fontFamily,
fontSize: fontSize,
fontWeight: fontWeight,
Expand Down
1 change: 0 additions & 1 deletion lib/web_ui/test/html/text/layout_fragmenter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import 'package:ui/ui.dart';
import '../paragraph/helper.dart';

final EngineTextStyle defaultStyle = EngineTextStyle.only(
color: const Color(0xFFFF0000),
fontFamily: FlutterViewEmbedder.defaultFontFamily,
fontSize: FlutterViewEmbedder.defaultFontSize,
);
Expand Down