Fix MarkupError crash when hovering a Markdown table cell with bracketed text - #6644
Open
jryyangjy wants to merge 1 commit into
Open
Fix MarkupError crash when hovering a Markdown table cell with bracketed text#6644jryyangjy wants to merge 1 commit into
jryyangjy wants to merge 1 commit into
Conversation
MarkdownTableContent.compose() set table-cell tooltips from cell.plain (a plain str), which is rendered with markup enabled. Cell text that happens to look like markup (e.g. contains "[...]") is then mis-parsed and raises MarkupError, crashing the app on hover. The streaming path (_update_rows) already passes the Content object directly, which is never markup-parsed. Do the same in compose() for consistency, matching how header tooltips are already handled. Fixes Textualize#6642
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.
Problem
MarkdownTableContent.compose()sets a table cell's tooltip fromcell.plain-- a plainstrextracted from the cell'sContent:Tooltips set from a
strare rendered with markup enabled (Static.update()->visualize(..., markup=self._render_markup)->Content.from_markup(obj), seetextual/visual.py). If the cell's text happens to contain something that looks like markup -- e.g.([sys:LOCK=SAFE…])-- hovering the cell raisesMarkupErrorand crashes the app.The streaming path (
_update_rows, used when aMarkdownwidget is fed incrementally viaget_stream) already avoids this by passing theContentobject directly:.with_tooltip(cell). BecauseContentalready implements theVisualprotocol,visualize()returns it unchanged and never re-parses it as markup. Header cell tooltips (.with_tooltip(header), a few lines above) already do this correctly too -- only the row-cell path incompose()had the bug.Verification
Reproduced the crash headlessly with
run_test()(no real terminal needed), using the exact repro from the issue:main: the bracketed cell's tooltip is a plainstr, and rendering it with markup enabled raisestextual.markup.MarkupError: Expected markup value (found '…]) persona override').Contentobject, and rendering it raises nothing.Fix
_markdown.py: passcellinstead ofcell.plaintowith_tooltip()in the one-shotcompose()path, matching the existing streaming path and the header-tooltip path.test_markdown_table_cell_tooltip_with_bracketed_text_does_not_crash) that mounts a one-shot markdown table with bracketed cell text and asserts every cell's tooltip renders (viavisualize(..., markup=True)) without raising.## Unreleased, following this repo's existing pattern for in-progress fixes.Testing
pytest tests/test_markdown.py: 26 passed (including the new regression test).pytest tests/snapshot_tests/test_snapshots.py -k markdown: 10 passed, 1 pre-existing failure (test_text_area_language_rendering[markdown], a TextArea syntax-highlighting snapshot unrelated to this change -- confirmed it fails identically on unmodifiedmain, likely a local font/terminal rendering mismatch against the reference snapshot).Fixes #6642