Skip to content

Fix broken image flash on bridge-managed uploads#1072

Open
mbarta wants to merge 2 commits into
mainfrom
mb/fix_bridge_img_preview
Open

Fix broken image flash on bridge-managed uploads#1072
mbarta wants to merge 2 commits into
mainfrom
mb/fix_bridge_img_preview

Conversation

@mbarta

@mbarta mbarta commented May 27, 2026

Copy link
Copy Markdown
Member

Bridge-managed uploads (uploadUrl is null) hand Lexxy a placeholder File with no real image bytes. $showUploadedAttachment still built a previewSrc from it via URL.createObjectURL, producing a blob: URL pointing at empty content that rendered as a broken image until #preloadAndSwapSrc swapped in the server URL.

Gate the local preview on uploadUrl != null — the same signal createDOM already uses — so bridge-materialized images point straight at the server URL instead of a broken blob.

Bridge-managed uploads (uploadUrl is null) hand Lexxy a placeholder File
with no real image bytes, so URL.createObjectURL produced a blob: URL
pointing at empty content that rendered as a broken image until the
server URL was swapped in. Gate the local previewSrc on uploadUrl, the
same signal createDOM already uses, so the img points straight at the
server URL instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 27, 2026 11:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mbarta mbarta self-assigned this May 27, 2026
@samuelpecher

Copy link
Copy Markdown
Collaborator

I think we'd be better off with ManagedAttachmentUploadNode or allowing an upload delegate for the upload node if there's this much divergence

@mbarta

mbarta commented May 27, 2026

Copy link
Copy Markdown
Member Author

I think we'd be better off with ManagedAttachmentUploadNode or allowing an upload delegate for the upload node if there's this much divergence

Thanks @samuelpecher, are you happy to take over? I can give it a go using Claude but it'd still require a review from a web developer.

The local DirectUpload and bridge-managed upload paths had diverged at
three behavioral points keyed on `uploadUrl != null` (preview source,
file-vs-image figure, whether to run DirectUpload). Make the split
explicit via a subclass:

- `ActionTextAttachmentUploadNode` now owns the local DirectUpload
  behavior and exposes two hooks (`canPreviewFileLocally`,
  `$buildPreviewSrcForBlob`) plus a non-private `$startUploadIfNeeded`.
- `ManagedAttachmentUploadNode` extends it and overrides those hooks
  for the bridge case: no local preview, no DirectUpload.

The bridge factory (`Contents#insertPendingAttachment`) creates the
managed subclass; the local factory still creates the base. The
mutation listener is registered for both so the uploads-busy validity
counter still catches bridge uploads.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@samuelpecher

Copy link
Copy Markdown
Collaborator

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.

3 participants