Skip to content

Only locate a quote in the text the bubble actually rendered - #3356

Open
FrayxRulez wants to merge 1 commit into
developfrom
highlight-quote-mismatch
Open

FrayxRulez wants to merge 1 commit into
developfrom
highlight-quote-mismatch

Conversation

@FrayxRulez

Copy link
Copy Markdown
Collaborator

NullReferenceException at MessageBubble.xaml.cs:2961, reported by crash telemetry on 12.9.1.

   0  Telegram.Controls.Messages.MessageBubble.Highlight
      Telegram/Controls/Messages/MessageBubble.xaml.cs:2961
   1  Telegram.Controls.Chats.ChatHistoryView.<ScrollToItem>g__GetHighlightArea|57_0
      Telegram/Controls/Chats/ChatHistoryView.cs:435
   2  Telegram.Controls.Chats.ChatHistoryView.<ScrollToItem>d__57.MoveNext
      Telegram/Controls/Chats/ChatHistoryView.cs:443

Cause

Highlight walks message.Text.Paragraphs and asks the view for the block that renders
each one:

StyledParagraph styled = message.Text.Paragraphs[j];
Paragraph paragraph = Message.GetBlock(j, out double width, out Point adjustment) as Paragraph;

It pairs an index into the model with an index into the view, and nothing guaranteed the two
describe the same text. MessageTextBlock.GetBlock returns null when no range covers the
index, and paragraph.Margin.Left a few lines down is the dereference that throws.

The assumption is known to be breakable elsewhere — TextPointer.OffsetToIndex bails out on
textBlock.Blocks.Count != text.Paragraphs.Count, and MessageQuote_Loaded already refuses to
build a quote for MessageRichMessage. Highlight is the only GetBlock caller with no check.

MessageAnimatedEmoji is the deterministic case. The underlying message is a plain text
message — TDLib re-types it as messageAnimatedEmoji in the API layer and can flip it live via
updateMessageContent — so it is quotable in every client. The quote rides on the replying
message and carries the original text, searchQuote compares text only and not entities, and
GetCaption() returns the emoji, so the quote always matches at index 0. But UpdateMessageText
excludes that content type from Message.SetText, and Recycle() clears the ranges, so every
GetBlock returns null.

Translation and summary reach the same state differently. Each is an independent
StyledText fed into Message while Highlight still walks message.Text. Entities are
dropped in translation, so a blockquote that forced a paragraph break in the original no longer
does and the rendered text can have fewer paragraphs than the model.

The fix

Compare the two by reference before entering the quote branch, rather than guarding the null it
produces:

if (options.Quote != null && options.Quote.IsManual && !string.IsNullOrEmpty(message.Text?.Text)
    && Message.Text == message.Text)

MessageTextBlock now exposes the StyledText its blocks were built from, mirroring
FormattedTextBlock.Text.

Guarding the null instead would have left a silent bug: the null only appears when the paragraph
counts differ. When they happen to match but the content does not — a translation of equal line
count — the old code drew the highlight rectangles in the wrong places rather than crashing.

Falling back to the whole-bubble rect is also the right visual here. light is true for
MessageAnimatedEmoji, so the fallback targets Media, which is what actually drew the content.

Nothing upstream can prevent the state: the quote already exists in history and may have been
made from another client, so it cannot be validated away at render time.

Build

Not built — a UWP/.NET Native build isn't available here. Both files pass
CSharpSyntaxTree.ParseText(...).GetDiagnostics(), which confirms they parse and nothing more.

🤖 Generated with Claude Code

MessageBubble.Highlight walks message.Text.Paragraphs and asks Message for the
block rendering each one, pairing an index into the model with an index into
the view. Nothing guaranteed the two describe the same text, and GetBlock
returns null when no range covers the index, so the paragraph dereference a few
lines later threw.

MessageAnimatedEmoji is the deterministic case. The message is a plain text
message that TDLib re-types in the API layer, so it is quotable everywhere and
the quote rides on the replying message; searchQuote compares text only and
GetCaption returns the emoji, so the quote always matches. But UpdateMessageText
excludes it from SetText and Recycle clears the ranges, so every GetBlock is
null. A translation or a summary reaches the same state differently: it is an
independent StyledText, and since entities are dropped in translation a
blockquote that forced a paragraph break in the original no longer does, so the
rendered text can have fewer paragraphs than the model.

Compare the two by reference before entering the branch, rather than guarding
the null it produces. The null only appears when the paragraph counts differ;
when they happen to match but the content does not, the old code drew the
highlight in the wrong places instead of crashing, and that is silent. Falling
back to the whole-bubble rect is also the right visual: content that never
reaches SetText is drawn by Media, which the fallback already targets.

Reported by crash telemetry on 12.9.1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant