Fix markdown cell not re-rendering when re-executed (#151151)#319247
Open
Tatlatat wants to merge 1 commit into
Open
Fix markdown cell not re-rendering when re-executed (#151151)#319247Tatlatat wants to merge 1 commit into
Tatlatat wants to merge 1 commit into
Conversation
A markdown cell with an `<img>` pointing to a missing file shows a broken image. After the file is created and the cell is re-executed/re-rendered without editing its text, the image stayed broken because the preview was not re-rendered when its content was unchanged. Thread a `forceRerender` flag from the view layer down to the webview: `MarkupCell` now forces a re-render whenever the cell leaves edit mode (`editStateChanged`) — covering Enter, the toolbar, and Escape — so that `showMarkupPreview` bypasses its unchanged-content short-circuit and the webview re-fetches resources such as `<img>` tags. Content/metadata-only, layout, scroll and selection changes are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds a “force re-render” path for markup previews so the preview is refreshed even when the markdown source/metadata is unchanged (e.g., to re-fetch resources like images after leaving edit mode).
Changes:
- Extend markup preview rendering APIs to accept a
forceRerenderboolean. - Trigger forced preview refresh when a markup cell’s edit state changes.
- Update webview-side diffing logic to bypass “same content/metadata” short-circuit when forced.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts | Adds forceRerender parameter and bypasses same-content checks to re-send render messages. |
| src/vs/workbench/contrib/notebook/browser/view/cellParts/markupCell.ts | Propagates an edit-state-derived forceRerender flag into preview updates. |
| src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts | Documents and threads forceRerender through createMarkupPreview to the webview layer. |
| src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts | Updates the editor delegate interface to accept optional forceRerender. |
Comment on lines
+135
to
+138
| // When the cell leaves edit mode (e.g. it was rendered/executed via Enter, the | ||
| // toolbar, or Escape) force the preview to re-render even if the markdown source is | ||
| // unchanged, so that resources such as <img> tags are re-fetched. See #151151. | ||
| this.viewUpdate(e.editStateChanged); |
| readonly onDidChangeOptions: Event<void>; | ||
| readonly onDidChangeDecorations: Event<void>; | ||
| createMarkupPreview(cell: ICellViewModel): Promise<void>; | ||
| createMarkupPreview(cell: ICellViewModel, forceRerender?: boolean): Promise<void>; |
Author
|
@microsoft-github-policy-service agree
Vào Thứ 2, 1 thg 6, 2026 lúc 09:54 Copilot ***@***.***> đã
viết:
… ***@***.**** commented on this pull request.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds a “force re-render” path for markup previews so the preview
is refreshed even when the markdown source/metadata is unchanged (e.g., to
re-fetch resources like images after leaving edit mode).
*Changes:*
- Extend markup preview rendering APIs to accept a forceRerender
boolean.
- Trigger forced preview refresh when a markup cell’s edit state
changes.
- Update webview-side diffing logic to bypass “same content/metadata”
short-circuit when forced.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and
generated 2 comments.
File Description
src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts Adds
forceRerender parameter and bypasses same-content checks to re-send
render messages.
src/vs/workbench/contrib/notebook/browser/view/cellParts/markupCell.ts Propagates
an edit-state-derived forceRerender flag into preview updates.
src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts Documents
and threads forceRerender through createMarkupPreview to the webview
layer.
src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts Updates the
editor delegate interface to accept optional forceRerender.
------------------------------
In src/vs/workbench/contrib/notebook/browser/view/cellParts/markupCell.ts
<#319247 (comment)>:
> + // When the cell leaves edit mode (e.g. it was rendered/executed via Enter, the
+ // toolbar, or Escape) force the preview to re-render even if the markdown source is
+ // unchanged, so that resources such as <img> tags are re-fetched. See #151151.
+ this.viewUpdate(e.editStateChanged);
The comment specifies forcing a re-render when the cell *leaves* edit
mode, but the code passes e.editStateChanged directly, which is true for
both entering and leaving edit mode. Consider deriving the flag from the
*current* edit state (e.g., only force when the new state is not
CellEditState.Editing), or adjust the comment to reflect the broader
behavior. This keeps intent and behavior aligned and avoids accidentally
expanding forced re-render triggers in future refactors.
------------------------------
In src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts
<#319247 (comment)>:
> @@ -836,7 +836,7 @@ export interface INotebookEditorDelegate extends INotebookEditor {
readonly creationOptions: INotebookEditorCreationOptions;
readonly onDidChangeOptions: Event<void>;
readonly onDidChangeDecorations: Event<void>;
- createMarkupPreview(cell: ICellViewModel): Promise<void>;
+ createMarkupPreview(cell: ICellViewModel, forceRerender?: boolean): Promise<void>;
The method name createMarkupPreview implies this should only accept
markup cells, but the signature allows any ICellViewModel. Since this
line is being changed anyway, consider narrowing the parameter type to the
most specific markup cell view model/interface available in this layer.
This prevents accidental calls with non-markup cells and makes the contract
clearer for implementers and callers.
—
Reply to this email directly, view it on GitHub
<#319247?email_source=notifications&email_token=CALUBKFQ5ZEJ6KLZDNLD5FD45TV7NA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMZZHAYTKMJUGIY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4398151421>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CALUBKB3TDS7UXFHYYR4CG345TV7NAVCNFSM6AAAAACZU5M5R2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DGOJYGE2TCNBSGE>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/CALUBKEVY7G4L65Q2CTJVJL45TV7NA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMZZHAYTKMJUGIY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/CALUBKFN322EY3MFCAR2RSL45TV7NA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMZZHAYTKMJUGIY2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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.
A markdown cell with an
<img>pointing to a missing file shows a broken image. After the file is created and the cell is re-executed/re-rendered without editing its text, the image stayed broken because the preview was not re-rendered when its content was unchanged.Thread a
forceRerenderflag from the view layer down to the webview:MarkupCellnow forces a re-render whenever the cell leaves edit mode (editStateChanged) — covering Enter, the toolbar, and Escape — so thatshowMarkupPreviewbypasses its unchanged-content short-circuit and the webview re-fetches resources such as<img>tags. Content/metadata-only, layout, scroll and selection changes are unaffected.