Skip to content

Serve media found by a custom theme under the media rules - #217

Open
snoopdave wants to merge 2 commits into
roller-6.1.xfrom
theme-resource-media-headers
Open

snoopdave wants to merge 2 commits into
roller-6.1.xfrom
theme-resource-media-headers

Conversation

@snoopdave

Copy link
Copy Markdown
Contributor

A weblog with a custom theme finds its resources in the media library. ResourceServlet and PreviewResourceServlet now serve those files with the same headers as other media:

  • Stylesheets and scripts keep their type, so a custom theme's pages can still load them.
  • Other types are served inline only when they are an image, audio, video, PDF or plain text. Anything else is a download, as on /mediaresource/.

The new MediaTypePolicy.applyThemeMediaHeaders makes that choice, and WeblogCustomTheme.isMediaFile tells the servlets which resources came from the media library. Resources that ship with a theme on disk are unchanged.

The upload form no longer sets a media file's original path. The media file editor sets it only when mediafile.originalPathEdit.enabled=true, which is when its form shows the field. Saving the editor without that setting used to clear the original path; it no longer does.

Tests:

  • ResourceServletTest: a custom-theme weblog serves an HTML media file as a download and a CSS media file as text/css.
  • MediaFileBeanTest: copyTo leaves the original path alone.
  • MediaTypePolicyTest, WeblogCustomThemeTest: the new methods.
  • CHANGES.md: behaviour-change note.

Full build and tests pass on JDK 11 (412 tests, 0 failures).

A weblog with a custom theme finds its resources in the media library.
Those files now get the same response headers as other media: stylesheets
and scripts keep their type so the theme's pages can load them, and other
types are served inline only when they are an image, audio, video, PDF or
plain text. Both ResourceServlet and PreviewResourceServlet apply this.

The upload form no longer sets a media file's original path. The media
file editor sets it only when mediafile.originalPathEdit.enabled is true,
which is when its form shows the field. Saving the editor without that
setting no longer clears the original path.
Comment thread CHANGES.md
library as before. Other files there, such as HTML or SVG, are served like
any other media file: as a download unless they are an image, audio, video,
PDF or plain text. The media file editor sets a file's original path only
when `mediafile.originalPathEdit.enabled=true`, as its form already shows.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah, the memories "mediafile.originalPathEdit.enabled".

@snoopdave

Copy link
Copy Markdown
Contributor Author

🐞Claude Issue: PR-Review: General Issues

These are outside the diff hunks:

  • Important: MediaCollection.java:155 (AtomPub media upload) still sets the original path, from the request path. This PR makes the original path something only theme import and the editor (when mediafile.originalPathEdit.enabled=true) set. The AtomPub value is also odd: justPath is the last path segment (the directory, for example /images), not the file's path, so every file uploaded to one directory gets the same original path, and getMediaFileByOriginalPath can then match more than one row. Suggest removing mf.setOriginalPath(justPath) there, or setting it the way ThemeManagerImpl does (dir + "/" + name) only if AtomPub clients need it.
  • Important: No test covers the new MediaFileEdit.save behaviour: the original path is kept when mediafile.originalPathEdit.enabled is false and set when it is true. PreviewResourceServlet has the same change as ResourceServlet but no test; a small PreviewResourceServletTest mirroring ResourceServletTest would cover both lookups (preview theme and weblog theme).

@snoopdave

Copy link
Copy Markdown
Contributor Author

🤖Claude: fixed both general issues (local changes, not yet pushed):

  • AtomPub: removed mf.setOriginalPath(justPath) from MediaCollection.postMedia, matching master after Fix AtomPub media naming, paging and XML output, and editor menus #213. CHANGES.md notes it.
  • Tests: added MediaFileEditTest (original path kept when mediafile.originalPathEdit.enabled is false, set when true) and PreviewResourceServletTest (preview-theme and weblog-theme lookups; HTML media is a download, CSS keeps its type). Both fail without the fix. Full suite on JDK 11: 417 tests, 0 failures, 1 skipped.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant