fix: guard link recognizer against stale/detached node in _tapNodeLink - #2753
Open
mtallenca wants to merge 1 commit into
Open
fix: guard link recognizer against stale/detached node in _tapNodeLink#2753mtallenca wants to merge 1 commit into
mtallenca wants to merge 1 commit into
Conversation
_TextLineState caches link GestureRecognizers in _linkRecognizers keyed by Node, but that map is only cleared on readOnly-toggle, meta-key change, or dispose -- never when the line content changes. The onTap/onLongPress closure captures a specific segment node and re-reads its link attribute when the gesture fires. If the document mutates (e.g. a note with many links re-rendered while navigating), the captured node can be detached or no longer carry a link attribute by the time the arena sweeps the tap, making node.style.attributes[Attribute.link.key]! throw a null-check crash. Read the link attribute defensively (?.value) in both _tapNodeLink and _longPressLink; _tapLink already no-ops on null.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
_TextLineStatecaches per-node link gesture recognizers in_linkRecognizers(Map<Node, GestureRecognizer>), and the recognizer's callback captures a specificsegmentnode:That map is only cleared on a
readOnlytoggle (didUpdateWidget), a meta/ctrl key change (_pressedKeysChanged), ordispose— never when the line's document content changes. So the cached recognizer (and the node it captured) can outlive a document mutation/re-render.When the gesture finally fires,
_tapNodeLink/_longPressLinkre-read the node's link attribute and force-unwrap it:If the document mutated between recognizer creation and the tap being swept by the gesture arena, the captured node can be detached or no longer carry a
linkattribute, soattributes[Attribute.link.key]isnulland the!throws.We see this crash in production (Crashlytics, flutter_quill 11.5.1, Android) on a read-only document with many links that is re-rendered while the user navigates — a queued tap is delivered during
GestureArenaManager.sweepagainst a now-stale recognizer:Fix
Read the link attribute defensively (
?.value) in both_tapNodeLinkand_longPressLinkinstead of force-unwrapping._tapLinkalready no-ops on anulllink;_longPressLinkgets an early return. When the captured node is stale, the gesture simply does nothing instead of crashing.Notes
This is a defensive guard against a timing/staleness window (stale cached recognizer fired during arena sweep after a document mutation), which is awkward to reproduce deterministically in a widget test. Happy to add a regression test if a maintainer can point me at the preferred way to drive a cached recognizer against a mutated node.