Skip to content

content: Fix bugs where TextSpan styles clobber each other with InlineContent.style attributes #1820

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

Merged
merged 9 commits into from
Aug 21, 2025
Merged
16 changes: 6 additions & 10 deletions lib/widgets/content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1127,8 +1127,7 @@ class _InlineContentBuilder {

case UnicodeEmojiNode():
return TextSpan(text: node.emojiUnicode, recognizer: _recognizer,
style: widget.style
.merge(ContentTheme.of(_context!).textStyleEmoji));
style: ContentTheme.of(_context!).textStyleEmoji);

case ImageEmojiNode():
return WidgetSpan(alignment: PlaceholderAlignment.middle,
Expand All @@ -1138,9 +1137,8 @@ class _InlineContentBuilder {
final nodes = node.nodes;
return nodes == null
? TextSpan(
style: widget.style
.merge(ContentTheme.of(_context!).textStyleInlineMath)
.apply(fontSizeFactor: kInlineCodeFontSizeFactor),
style: ContentTheme.of(_context!).textStyleInlineMath
.copyWith(fontSize: widget.style.fontSize! * kInlineCodeFontSizeFactor),
children: [TextSpan(text: node.texSource)])
: WidgetSpan(
alignment: PlaceholderAlignment.baseline,
Expand Down Expand Up @@ -1181,11 +1179,9 @@ class _InlineContentBuilder {
// TODO `code`: find equivalent of web's `unicode-bidi: embed; direction: ltr`

return _buildNodes(
style: widget.style
.merge(ContentTheme.of(_context!).textStyleInlineCode)
.apply(fontSizeFactor: kInlineCodeFontSizeFactor),
node.nodes,
);
style: ContentTheme.of(_context!).textStyleInlineCode
.copyWith(fontSize: widget.style.fontSize! * kInlineCodeFontSizeFactor),
node.nodes);

// Another fun solution -- we can in fact have a border! Like so:
// TextStyle(
Expand Down
16 changes: 8 additions & 8 deletions lib/widgets/text.dart
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,11 @@ double bolderWght(double baseWght, {double by = 300}) {
return clampDouble(baseWght + by, kWghtMin, kWghtMax);
}

/// A [TextStyle] whose [FontVariation] "wght" and [TextStyle.fontWeight]
/// have been raised using [bolderWght].
/// A [TextStyle] with [FontVariation] "wght" and [TextStyle.fontWeight]
/// that have been raised from the input using [bolderWght].
///
/// Non-weight attributes in [style] are ignored
/// and will not appear in the result.
///
/// [style] must have already been processed with [weightVariableTextStyle],
/// and [by] must be positive.
Expand All @@ -311,12 +314,9 @@ TextStyle bolderWghtTextStyle(TextStyle style, {double by = 300}) {

final newWght = bolderWght(wghtFromTextStyle(style)!, by: by);

TextStyle result = style.copyWith(
fontVariations: style.fontVariations!.map((v) => v.axis == 'wght'
? FontVariation('wght', newWght)
: v).toList(),
fontWeight: clampVariableFontWeight(newWght),
);
TextStyle result = TextStyle(
fontVariations: [FontVariation('wght', newWght)],
fontWeight: clampVariableFontWeight(newWght));

assert(() {
result = result.copyWith(debugLabel: 'bolderWghtTextStyle(by: $by)');
Expand Down
12 changes: 8 additions & 4 deletions test/model/content_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ class ContentExample {
'<p><strong>bold</strong></p>',
const StrongNode(nodes: [TextNode('bold')]));

static final deleted = ContentExample.inline(
'deleted/strike-through',
'~~strike through~~',
expectedText: 'strike through',
'<p><del>strike through</del></p>',
const DeletedNode(nodes: [TextNode('strike through')]));

static final emphasis = ContentExample.inline(
'emphasis/italic',
'*italic*',
Expand Down Expand Up @@ -1535,10 +1542,7 @@ void main() async {

testParseExample(ContentExample.strong);

testParseInline('parse deleted/strike-through',
// "~~strike through~~"
'<p><del>strike through</del></p>',
const DeletedNode(nodes: [TextNode('strike through')]));
testParseExample(ContentExample.deleted);

testParseExample(ContentExample.emphasis);

Expand Down
84 changes: 63 additions & 21 deletions test/widgets/content_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,11 @@ void main() {
/// [styleFinder] must return the [TextStyle] containing the "wght"
/// (in [TextStyle.fontVariations]) and the [TextStyle.fontWeight]
/// to be checked.
Future<void> testFontWeight(String description, {
void testFontWeight(String description, {
required Widget content,
required double expectedWght,
required TextStyle Function(WidgetTester tester) styleFinder,
}) async {
}) {
for (final platformRequestsBold in [false, true]) {
testWidgets(
description + (platformRequestsBold ? ' (platform requests bold)' : ''),
Expand Down Expand Up @@ -707,8 +707,18 @@ void main() {
'<tbody>\n<tr>\n<td>text</td>\n</tr>\n</tbody>\n'
'</table>'),
styleFinder: findWordBold);

testWidgets('has strike-through line in strike-through', (tester) async {
// Regression test for: https://github.com/zulip/zulip-flutter/issues/1817
await prepareContent(tester,
plainContent('<p><del><strong>bold</strong></del></p>'));
final style = mergedStyleOf(tester, 'bold');
check(style!.decoration).equals(TextDecoration.lineThrough);
});
});

testContentSmoke(ContentExample.deleted);

testContentSmoke(ContentExample.emphasis);

group('inline code', () {
Expand All @@ -719,6 +729,22 @@ void main() {
targetHtml: '<code>code</code>',
targetFontSizeFinder: mkTargetFontSizeFinderFromPattern('code'));
});

testFontWeight('is bold in bold span',
// Regression test for: https://github.com/zulip/zulip-flutter/issues/1812
expectedWght: 600,
// **`bold`**
content: plainContent('<p><strong><code>bold</code></strong></p>'),
styleFinder: (tester) => mergedStyleOf(tester, 'bold')!,
);

testWidgets('is link-colored in link span', (tester) async {
// Regression test for: https://github.com/zulip/zulip-flutter/issues/806
await prepareContent(tester,
plainContent('<p><a href="https://example/"><code>code</code></a></p>'));
final style = mergedStyleOf(tester, 'code');
check(style!.color).equals(const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor());
});
});

group('UserMention', () {
Expand Down Expand Up @@ -980,6 +1006,14 @@ void main() {
_ => throw StateError('unexpected platform in test'),
});
}, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));

testWidgets('has strike-through line in strike-through', (tester) async {
// Regression test for https://github.com/zulip/zulip-flutter/issues/1818
await prepareContent(tester,
plainContent('<p><del>foo<span aria-label="thumbs up" class="emoji emoji-1f44d" role="img" title="thumbs up">:thumbs_up:</span>bar</del></p>'));
final style = mergedStyleOf(tester, '\u{1f44d}');
check(style!.decoration).equals(TextDecoration.lineThrough);
});
});

group('inline math', () {
Expand Down Expand Up @@ -1010,27 +1044,35 @@ void main() {
});
});

testWidgets('maintains font-size ratio with surrounding text, when falling back to TeX source', (tester) async {
const unsupportedHtml = '<span class="katex">'
'<span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>λ</mi></mrow>'
'<annotation encoding="application/x-tex"> \\lambda </annotation></semantics></math></span>'
'<span class="katex-html" aria-hidden="true">'
'<span class="base unknown">' // Server doesn't generate this 'unknown' class.
'<span class="strut" style="height:0.6944em;"></span>'
'<span class="mord mathnormal">λ</span></span></span></span>';
await checkFontSizeRatio(tester,
targetHtml: unsupportedHtml,
targetFontSizeFinder: mkTargetFontSizeFinderFromPattern(r'\lambda'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: squash these together:

9cc505f content test [nfc]: Get unsupported-katex HTML from ContentExample
bf823e1 content test [nfc]: Pull out an expectedText variable from ContentExample

since the first one leaves the literal r'\lambda' unexplained

});
group('fallback to displaying KaTeX source if unsupported KaTeX HTML', () {
testContentSmoke(ContentExample.mathInlineUnknown);

testWidgets('displays KaTeX content', (tester) async {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Looks like the behavior of that test differs from this one, just in
not passing `findRichText: true`. But that didn't stop it from
finding the expected test and passing, so, shrug.

s/expected test/expected text/ ?

await prepareContent(tester, plainContent(ContentExample.mathInline.html));
tester.widget(find.text('λ', findRichText: true));
});
assert(ContentExample.mathInlineUnknown.html.startsWith('<p>'));
assert(ContentExample.mathInlineUnknown.html.endsWith('</p>'));
final unsupportedKatexHtml = ContentExample.mathInlineUnknown.html
.substring(3, ContentExample.mathInlineUnknown.html.length - 4);
final expectedText = ContentExample.mathInlineUnknown.expectedText!;

testWidgets('fallback to displaying KaTeX source if unsupported KaTeX HTML', (tester) async {
await prepareContent(tester, plainContent(ContentExample.mathInlineUnknown.html));
tester.widget(find.text(r'\lambda'));
testWidgets('maintains font-size ratio with surrounding text, when falling back to TeX source', (tester) async {
await checkFontSizeRatio(tester,
targetHtml: unsupportedKatexHtml,
targetFontSizeFinder: mkTargetFontSizeFinderFromPattern(expectedText));
});

testFontWeight('is bold in bold span',
// Regression test for: https://github.com/zulip/zulip-flutter/issues/1812
expectedWght: 600,
content: plainContent('<p><strong>$unsupportedKatexHtml</strong></p>'),
styleFinder: (tester) => mergedStyleOf(tester, expectedText)!,
);

testWidgets('is link-colored in link span', (tester) async {
// Regression test for: https://github.com/zulip/zulip-flutter/issues/806
await prepareContent(tester,
plainContent('<p><a href="https://example/">$unsupportedKatexHtml</a></p>'));
final style = mergedStyleOf(tester, expectedText);
check(style!.color).equals(const HSLColor.fromAHSL(1, 200, 1, 0.4).toColor());
});
});
});

Expand Down