Skip to content

Frontend iframe renderer framework: 3D models, OpenAPI#37233

Merged
silverwind merged 39 commits intogo-gitea:mainfrom
silverwind:3diframe
Apr 17, 2026
Merged

Frontend iframe renderer framework: 3D models, OpenAPI#37233
silverwind merged 39 commits intogo-gitea:mainfrom
silverwind:3diframe

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 15, 2026

Introduces a frontend external-render framework that runs renderer plugins inside an iframe (loaded via srcdoc to keep the CSP sandbox directive working without origin-related console noise), and migrates the 3D viewer and OpenAPI/Swagger renderers onto it. PDF and asciicast paths are refactored to share the same data-render-name mechanism.

Adds e2e coverage for 3D, PDF, asciicast and OpenAPI render paths, plus a regression for the RefTypeNameSubURL double-escape on non-ASCII branch names.

image

This PR was written with the help of Claude Opus 4.7

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 15, 2026
@silverwind silverwind added the backport/v1.26 This PR should be backported to Gitea 1.26 label Apr 15, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

But the problem is, the file extensions are ambiguous

Without frontend render's trial, backend doesn't know whether the file is really renderable.

That's why the old code "tries to render the 3d model, if fails, then hides the rendered button"

I think "Use Content-Seucrity-Policy: script nonce #37232" is good enough and safe enough (safer)

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 15, 2026

Without frontend render's trial, backend doesn't know whether the file is really renderable.

I tested using a broken file and it rendered an error message. Is that not acceptable?

@wxiaoguang
Copy link
Copy Markdown
Contributor

Without frontend render's trial, backend doesn't know whether the file is really renderable.

I tested using a broken file and it rendered an error message? Is that not acceptable?

It means the existing logic is broken, but I don't think it means backend should blindly treat the plenty of potential ambiguous file extensions as 3D model.

@silverwind
Copy link
Copy Markdown
Member Author

Maybe that trial could be restored, but it will be more complicated.

@wxiaoguang
Copy link
Copy Markdown
Contributor

image

@silverwind
Copy link
Copy Markdown
Member Author

I guess it would need a magic-byte detection in backend but given the numerous formats supported here, it would be complex.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 15, 2026

I guess it would need a magic-byte detection in backend but given the numerous formats supported here, it would be complex.

Yes, that's why the "detection" work was designed to be deferred to the renders.

Backend can't be (and shouldn't be) a knowing-everything detector in this case (if we need frontend renders)

@wxiaoguang
Copy link
Copy Markdown
Contributor

In my mind, the complete design is like this (if the iframe will be used):

  1. parent collect the supported extensions of frontend renders
  2. if the current file matches any extension, start the general iframe embedded render, and pass the file name & data via postMessage (iframe can't read the file due to different origin)
  3. the iframe calls the frontend renders one by one, and report the render result to parent by postMessage

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 15, 2026

So basically do the iframe rendering in frontend, like mermaid does it, with no separate vite entry point script.

@wxiaoguang
Copy link
Copy Markdown
Contributor

So basically do the iframe rendering in frontend,

I think so, otherwise I don't see how we could resolve the more potential conflicts in the future if there are more frontend renders and when they declare they can render the same extensions.

Unless we are sure that we can do everything in backend and/or there would be no any extension conflict.

like mermaid does it, with no separate vite entry point script.

IMO no, it's not like mermaid. mermaid is fully rendered in the same origin, it shares the parent's scripts.

But for all other general 3rd frontend renders, I think we should make them in different origins.

@silverwind silverwind marked this pull request as draft April 15, 2026 11:09
Comment thread web_src/js/viewer-3d.ts Outdated
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Apr 15, 2026

By the way, since the "broken file render logic" has been broken for a long time and nobody complains (#37233 (comment))

I think we can backport this as long as it doesn't make the situation worse


The plan for the future (#37233 (comment)) can be written in the comment as a TODO and leave the refactoring to the future (if anyone complains, or we need to add more frontend renders)

Comment thread vite.config.ts Outdated
wxiaoguang and others added 2 commits April 15, 2026 19:39
The backend renderer cannot reliably determine whether an ambiguous
extension (*.obj, *.off, *.step, etc.) is actually a 3D model without a
render trial, so detection is deferred to the frontend per PR discussion.

The plugin now fetches the file in the parent and ships bytes to a
sandboxed (null-origin) iframe via postMessage. Background and default
colors are read from live CSS and passed in, so the viewer matches the
current theme instead of hardcoded values.

The raw-file prompt is hidden while loading to avoid a "View Raw" flash,
and the container height is set via CSS so the loading spinner has the
same dimensions as the final rendered viewer.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind marked this pull request as ready for review April 15, 2026 18:01
@silverwind
Copy link
Copy Markdown
Member Author

All done, and this renderer also now has a e2e test.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@wxiaoguang wxiaoguang marked this pull request as draft April 17, 2026 19:37
auto-merge was automatically disabled April 17, 2026 19:37

Pull request was converted to draft

@silverwind silverwind removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 17, 2026
@silverwind
Copy link
Copy Markdown
Member Author

Given that padding/color is such a recurring issue, should add a assertion in the e2e for it. I can do after your fixes.

@wxiaoguang
Copy link
Copy Markdown
Contributor

Should be right now.

Before: the external render helper can read the query parameter directly.

After using "srcdoc": the query parameter can only be passed via the script's data attribute

@wxiaoguang wxiaoguang changed the title Frontend iframe renderer framework: 3D models, OpenAPI, PDF, asciicast Frontend iframe renderer framework: 3D models, OpenAPI Apr 17, 2026
@silverwind
Copy link
Copy Markdown
Member Author

Will re-verify all and add e2e assertions in a bit, currently branch is occupied with another change and I don't wanna do a worktree right now 😆.

Comment thread web_src/js/render/plugins/frontend-viewer-3d.ts
- assertFlushWithParent helper: child has no horizontal inset from
  parent — catches padding/border anywhere in between
- 3D, openapi, pdf, external tests assert iframe/container is flush
  with .file-view (regression for the .markup-class padding)
- 3D test asserts iframe body bgcolor matches parent body bgcolor
- eslint: add browser globals to e2e test config so page.evaluate
  callbacks don't trip no-undef

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 17, 2026

e2e tests added for padding and 3d bg color. renderers manually tested.

@silverwind silverwind marked this pull request as ready for review April 17, 2026 21:46
@silverwind silverwind enabled auto-merge (squash) April 17, 2026 21:46
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 17, 2026
Copy link
Copy Markdown
Member

@bircni bircni left a comment

Choose a reason for hiding this comment

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

lgtm

Throw on null narrows via control-flow analysis instead.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind merged commit d5831b9 into go-gitea:main Apr 17, 2026
26 checks passed
@silverwind silverwind deleted the 3diframe branch April 17, 2026 22:30
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 17, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 17, 2026
@lunny lunny mentioned this pull request Apr 17, 2026
wxiaoguang pushed a commit to wxiaoguang/gitea that referenced this pull request Apr 18, 2026
wxiaoguang added a commit that referenced this pull request Apr 18, 2026
Backport

* #37233
* #37272

---------

Co-authored-by: silverwind <me@silverwind.io>
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 18, 2026
Move the post-build manifest-patching work in `iifePlugin` from the
`closeBundle` hook to `writeBundle`. `closeBundle` fires via
`bundle.close()` in Vite's `finally` block, so a throw there replaces
the real compile error from `catch`. `writeBundle` runs only on a
successful `bundle.write()`, so the manifest is guaranteed to exist and
a throw propagates cleanly. This lets us drop the defensive try/catch
that silently let builds succeed with a stale manifest (flagged by
Copilot in go-gitea#37233 and by the FIXME comment left behind).

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot
Copy link
Copy Markdown
Collaborator

I was unable to create a backport for 1.26. @silverwind, please send one manually. 🍵

go run ./contrib/backport 37233
...  // fix git conflicts if any
go run ./contrib/backport --continue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/done All backports for this PR have been created backport/v1.26 This PR should be backported to Gitea 1.26 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/code-linting type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants