From 1a54eec3dbfd049f98dfca678c3060c5c3a8f0a1 Mon Sep 17 00:00:00 2001 From: Mohammad Asfour Date: Wed, 21 Oct 2026 13:24:32 -0400 Subject: [PATCH] fix(mobile): keep token labels visible in authored Markdown links (#7545) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary On iOS, an authored Markdown link renders as blank when its label is recognized as a Buzz channel, mention, or custom emoji. For example: ```markdown [#2959](https://example.com/) ``` The surrounding text remains visible, but the link has no visible label or tap target. Buzz's `_ChannelLinkMd`, `_MentionMd`, and `CustomEmojiMd` components return `WidgetSpan`s. `gpt_markdown` also renders an authored link through a `WidgetSpan` and reparses its label through the registered components. Because the Buzz components were allowed to run inside link labels, the result was a placeholder nested inside another placeholder, which does not paint on iOS. This PR adds the same scope restriction used by `gpt_markdown`'s built-in link component: ```dart @override Set get scopes => MarkdownComponent.allScopesExceptLinkLabel; ``` The override is applied to all three widget-based components. Authored labels such as `[#2959](url)`, `[@Alice](url)`, and `[:wave:](url)` therefore remain ordinary link text, while the same tokens outside links continue rendering as Buzz pills and custom emoji. The PR also updates the shared test helpers to find `RichText` subclasses. Since the upgrade to `gpt_markdown` 1.2.1, body paragraphs can render through `BidiRichText`; `find.byType(RichText)` does not match that subclass and could allow an absence assertion to pass without inspecting the message text. The handling of unknown bare `#tokens` is deliberately unchanged. Whether those should remain non-interactive pills or become plain text is a separate product decision. ### Related issue Fixes #6124. This supersedes #6135. That PR solves the same issue by maintaining a custom fork of `gpt_markdown`'s link component; this PR instead uses the scoped-component API already available in `gpt_markdown` 1.2.1. ### Testing Focused suites: ```bash flutter test \ test/features/channels/message_content_test.dart \ test/features/channels/message_content_custom_emoji_test.dart ``` Result: 95 tests passed, 0 failed. Additional checks: - `just mobile-check` — passed; analyzer reported no issues. - `just mobile-fmt` — passed; 549 files formatted with 0 changes. - Six new regression tests cover channel, mention, emoji, plain-link control, and outside-link behavior. - Removing the `_ChannelLinkMd` override causes two regression tests to fail; removing `_MentionMd` or `CustomEmojiMd` causes one test to fail in each case. - Three tests in `message_content_test.dart` directly assert that the nested-placeholder structure is absent. The custom-emoji tests separately verify literal text and the absence or presence of `CustomEmojiImage`, depending on whether the shortcode is inside or outside a link. The complete local mobile suite reports 15 failures that reproduce identically on a clean checkout of `main` at `44316ff7`. In [CI for that same commit](https://github.com/block/buzz/actions/runs/34238736818), `Clients / Mobile`, `Mobile`, and `Mobile Swift Domain / Mobile Swift` all passed. No native-device screenshot is included. The nested-placeholder fix was verified through the rendered widget structure, but an iOS simulator build was blocked by the existing `MLImage.framework` arm64-simulator incompatibility. Signed-off-by: Mohammad Asfour Signed-off-by: Samin Yeasar Sohag Co-authored-by: Samin Yeasar Sohag --- .../features/channels/message_content.dart | 7 + .../channels/message_content/token_pill.dart | 7 + .../custom_emoji/custom_emoji_render.dart | 7 + .../message_content_custom_emoji_test.dart | 28 ++++ .../channels/message_content_test.dart | 131 +++++++++++++++++- 5 files changed, 176 insertions(+), 4 deletions(-) diff --git a/mobile/lib/features/channels/message_content.dart b/mobile/lib/features/channels/message_content.dart index ca4b974415..1c7d4df483 100644 --- a/mobile/lib/features/channels/message_content.dart +++ b/mobile/lib/features/channels/message_content.dart @@ -1046,6 +1046,13 @@ class _MentionMd extends InlineMd { this.onMentionTap, }); + /// Excluded from link labels: this component renders a [WidgetSpan], and a + /// placeholder nested inside the link's own placeholder does not paint on + /// iOS — an authored `[@mention](url)` renders as nothing. Link resolution + /// wins over token detection inside a label. + @override + Set get scopes => MarkdownComponent.allScopesExceptLinkLabel; + @override RegExp get exp => _exp; diff --git a/mobile/lib/features/channels/message_content/token_pill.dart b/mobile/lib/features/channels/message_content/token_pill.dart index 5d369d241b..295bfb6a9a 100644 --- a/mobile/lib/features/channels/message_content/token_pill.dart +++ b/mobile/lib/features/channels/message_content/token_pill.dart @@ -18,6 +18,13 @@ class _ChannelLinkMd extends InlineMd { _ChannelLinkMd({required this.channelNames, this.onChannelTap}); + /// Excluded from link labels: this component renders a [WidgetSpan], and a + /// placeholder nested inside the link's own placeholder does not paint on + /// iOS — an authored `[#channel](url)` renders as nothing. Link resolution + /// wins over token detection inside a label. + @override + Set get scopes => MarkdownComponent.allScopesExceptLinkLabel; + @override RegExp get exp => _exp; diff --git a/mobile/lib/shared/custom_emoji/custom_emoji_render.dart b/mobile/lib/shared/custom_emoji/custom_emoji_render.dart index bbfdd25603..d193f006c0 100644 --- a/mobile/lib/shared/custom_emoji/custom_emoji_render.dart +++ b/mobile/lib/shared/custom_emoji/custom_emoji_render.dart @@ -65,6 +65,13 @@ class CustomEmojiMd extends InlineMd { this.size = kCustomEmojiInlineSize, }) : _urlByShortcode = _referencedUrls(palette, content); + /// Excluded from link labels: this component renders a [WidgetSpan], and a + /// placeholder nested inside the link's own placeholder does not paint on + /// iOS — an authored `[:emoji:](url)` renders as nothing. Link resolution + /// wins over token detection inside a label. + @override + Set get scopes => MarkdownComponent.allScopesExceptLinkLabel; + // Look ahead so adjacent tokens sharing a colon are both considered: // :unknown:known: must still allow the known token to match. static final _shortcodeScan = RegExp( diff --git a/mobile/test/features/channels/message_content_custom_emoji_test.dart b/mobile/test/features/channels/message_content_custom_emoji_test.dart index c43ed614c6..f43788c5ee 100644 --- a/mobile/test/features/channels/message_content_custom_emoji_test.dart +++ b/mobile/test/features/channels/message_content_custom_emoji_test.dart @@ -37,6 +37,34 @@ Widget _testable(String content, {List> tags = const []}) { } void main() { + // Regression: https://github.com/block/buzz/issues/6124 + // + // A custom-emoji WidgetSpan inside a link's own WidgetSpan does not paint on + // iOS, so the whole link renders as nothing. `CustomEmojiMd` opts out of + // `MarkdownScope.linkLabel`; this fails if that override is removed. + testWidgets('an emoji shortcode in a link label stays link text', ( + tester, + ) async { + await tester.pumpWidget(_testable('Say [:wave:](https://example.com/x)')); + + expect(find.byType(CustomEmojiImage), findsNothing); + final text = tester + .widgetList( + find.byWidgetPredicate((widget) => widget is RichText), + ) + .map((widget) => widget.text.toPlainText()) + .join(); + expect(text, contains(':wave:')); + }); + + testWidgets('an emoji shortcode outside a link label still renders', ( + tester, + ) async { + await tester.pumpWidget(_testable('Say :wave: now')); + + expect(find.byType(CustomEmojiImage), findsOneWidget); + }); + testWidgets('message wiring excludes unrelated emoji from the regex', ( tester, ) async { diff --git a/mobile/test/features/channels/message_content_test.dart b/mobile/test/features/channels/message_content_test.dart index 3715f8cf97..e5f9de0070 100644 --- a/mobile/test/features/channels/message_content_test.dart +++ b/mobile/test/features/channels/message_content_test.dart @@ -295,9 +295,49 @@ bool _isImageViewerHeroEnabled(WidgetTester tester) { return tester.widget(_imageViewerHeroMode()).enabled; } +/// Whether any placeholder span in the tree contains a further placeholder. +/// +/// This is the shape that renders as nothing on iOS: a token component's +/// `WidgetSpan` nested inside the `WidgetSpan` a link is already drawn in. +/// Each placeholder becomes its own paragraph, so a paragraph that both sits +/// under another paragraph's placeholder and holds a placeholder of its own is +/// the defect. +bool _hasNestedPlaceholder(WidgetTester tester) { + for (final rich in tester.widgetList(_anyRichText())) { + var placeholders = 0; + rich.text.visitChildren((span) { + if (span is PlaceholderSpan) placeholders++; + return true; + }); + if (placeholders == 0) continue; + // A paragraph holding a placeholder is fine on its own — the message body + // does that for every link. It is only a defect when that paragraph is + // itself inside another paragraph's placeholder. + final ancestors = find.ancestor( + of: find.byWidget(rich), + matching: _anyRichText(), + ); + if (ancestors.evaluate().isNotEmpty) return true; + } + return false; +} + +/// Matches every `RichText` in the tree, including subclasses. +/// +/// gpt_markdown renders body paragraphs through `BidiRichText`, a `RichText` +/// subclass. `find.byType` matches the exact runtime type only, so it silently +/// skips those paragraphs — an assertion that text is rendered then fails, and +/// an assertion that text is absent then passes for the wrong reason. +Finder _anyRichText() { + return find.byWidgetPredicate( + (widget) => widget is RichText, + description: 'any RichText (including subclasses)', + ); +} + /// Extracts all plain text from all RichText widgets in the tree. String _allRichText(WidgetTester tester) { - final richTexts = tester.widgetList(find.byType(RichText)); + final richTexts = tester.widgetList(_anyRichText()); return richTexts.map((rt) => rt.text.toPlainText()).join('\n'); } @@ -312,7 +352,7 @@ Finder _findRich(String text) { /// Checks that the given text appears as bold (fontWeight >= w600) in some /// TextSpan within any RichText widget. bool _hasBoldSpan(WidgetTester tester, String text) { - for (final rt in tester.widgetList(find.byType(RichText))) { + for (final rt in tester.widgetList(_anyRichText())) { if (_spanHasStyle( rt.text, text, @@ -326,7 +366,7 @@ bool _hasBoldSpan(WidgetTester tester, String text) { } bool _hasItalicSpan(WidgetTester tester, String text) { - for (final rt in tester.widgetList(find.byType(RichText))) { + for (final rt in tester.widgetList(_anyRichText())) { if (_spanHasStyle(rt.text, text, (s) => s.fontStyle == FontStyle.italic)) { return true; } @@ -335,7 +375,7 @@ bool _hasItalicSpan(WidgetTester tester, String text) { } bool _hasStrikethroughSpan(WidgetTester tester, String text) { - for (final rt in tester.widgetList(find.byType(RichText))) { + for (final rt in tester.widgetList(_anyRichText())) { if (_spanHasStyle( rt.text, text, @@ -3045,6 +3085,89 @@ Photos }); }); + // Regression: https://github.com/block/buzz/issues/6124 + // + // gpt_markdown renders a link's label by recursing with + // `MarkdownScope.linkLabel`. A token component that claims the label + // returns a WidgetSpan, which then sits inside the link's own WidgetSpan + // — and a placeholder nested in a placeholder does not paint on iOS, so + // the whole link disappears. The components opt out of that scope; these + // tests fail if any of the `scopes` overrides is removed. + group('authored link labels', () { + testWidgets('a #channel label stays link text, not a channel pill', ( + tester, + ) async { + await tester.pumpWidget( + _testable( + const MessageContent( + content: 'See [#2959](https://example.com/x) for details.', + // Known name and generic token both reach the component; use the + // known one so the pill would definitely render if not excluded. + channelNames: {'2959': 'ch-id-1'}, + ), + ), + ); + + expect(_findRich('#2959'), findsOneWidget); + expect(find.byIcon(BuzzIcons.hash), findsNothing); + expect(_hasNestedPlaceholder(tester), isFalse); + }); + + testWidgets('an @mention label stays link text, not a mention pill', ( + tester, + ) async { + await tester.pumpWidget( + _testable( + const MessageContent( + content: 'Ask [@Alice](https://example.com/x) about it.', + mentionNames: {'pk1': 'Alice'}, + ), + ), + ); + + expect(_findRich('@Alice'), findsOneWidget); + // The pill splits the label into a separate '@' and name; the link + // must not. + expect(find.text('@'), findsNothing); + expect(_hasNestedPlaceholder(tester), isFalse); + }); + + testWidgets('a label with no token renders unchanged', (tester) async { + await tester.pumpWidget( + _testable( + const MessageContent( + content: 'See [ticket 2959](https://example.com/x) for details.', + channelNames: {'2959': 'ch-id-1'}, + ), + ), + ); + + expect(_findRich('ticket 2959'), findsOneWidget); + expect(_hasNestedPlaceholder(tester), isFalse); + }); + + testWidgets('tokens outside a link label still render as pills', ( + tester, + ) async { + await tester.pumpWidget( + _testable( + const MessageContent( + content: 'See #2959 and @Alice, plus [#2959](https://x.test/y).', + channelNames: {'2959': 'ch-id-1'}, + mentionNames: {'pk1': 'Alice'}, + ), + ), + ); + + // Excluding the link-label scope must not disable the components + // everywhere else: the bare tokens keep their pills. + expect(find.byIcon(BuzzIcons.hash), findsOneWidget); + expect(find.text('@'), findsOneWidget); + expect(find.text('Alice'), findsOneWidget); + expect(_findRich('#2959'), findsOneWidget); + }); + }); + group('mixed content', () { testWidgets('renders bold with mentions', (tester) async { await tester.pumpWidget(