Restrict previewable image types and add config#1082
Open
samuelpecher wants to merge 1 commit into
Open
Conversation
Match Trix's previewable image types for determining what can be previewed in-browser and expect a preview from the server. Only image corresponding to the types will be galleryable. Files can still be replaced by a preview when the server sends `previewable: true` in the Blob attributes.
samuelpecher
requested review from
Copilot,
flavorjones,
jorgemanrubia and
zoltanhosszu
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restricts which image MIME types Lexxy treats as “previewable images” (i.e., eligible for in-browser image preview and image gallery membership), and introduces a global configuration option to override that detection pattern.
Changes:
- Add a configurable
attachmentPreviewablePattern(with a new default regex) to determine which content types are treated as previewable images. - Update uploader/gallery logic to rely on the attachment node’s previewable-image detection (instead of a generic helper).
- Add Playwright coverage around PSD/TIFF behavior (file-rendering, gallery exclusion, and server-side
previewablehandling).
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/browser/tests/attachments/psd_and_tiff.test.js | Adds browser tests ensuring PSD/TIFF don’t become gallery images by default, while still supporting server-side previews. |
| test/browser/helpers/active_storage_mock.js | Extends the ActiveStorage mock to optionally mark additional MIME types as previewable. |
| src/nodes/action_text_attachment_node.js | Introduces the default previewable-image regex and makes previewability configurable via global config. |
| src/helpers/html_helper.js | Removes the old generic isPreviewableImage helper to centralize previewability logic. |
| src/editor/contents/uploader.js | Switches multi-image upload detection to use the attachment node’s previewable-image logic. |
| docs/configuration.md | Documents the new attachmentPreviewablePattern global option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+81
to
+83
| static isPreviewableImage(contentType) { | ||
| return contentType.match(this.PREVIEWABLE_PATTERN) | ||
| } |
| - `attachmentTagName`: The tag name used for [Action Text custom attachments](https://guides.rubyonrails.org/action_text_overview.html#signed-globalid). By default, they will be rendered as `action-text-attachment` tags. | ||
| - `attachmentContentTypeNamespace`: The default content_type namespace for prompts. The default is `actiontext` which will result in `application/vnd.actiontext.[type]`. | ||
| - `attachmentPreviewablePattern`: A RegExp identifying which content types are previewable images, and therefore valid gallery children. For other file types, the server can return `{ previewable: true }` in a blob's attributes to show a preview. The default is `/^image(\/(avif|hei[cf]|gif|png|jpeg|webp)|$)/` | ||
| - `authenticatedUploads`: will set `withCredentials: true` for ActiveStorage upload requests if you are using authenticated upload contollers. Be sure to set cookie domain and server CORS/CSRF options accordingly. |
Collaborator
Author
|
cc @lylo as I know you've got an image-focused implementation |
|
|
||
| - `attachmentTagName`: The tag name used for [Action Text custom attachments](https://guides.rubyonrails.org/action_text_overview.html#signed-globalid). By default, they will be rendered as `action-text-attachment` tags. | ||
| - `attachmentContentTypeNamespace`: The default content_type namespace for prompts. The default is `actiontext` which will result in `application/vnd.actiontext.[type]`. | ||
| - `attachmentPreviewablePattern`: A RegExp identifying which content types are previewable images, and therefore valid gallery children. For other file types, the server can return `{ previewable: true }` in a blob's attributes to show a preview. The default is `/^image(\/(avif|hei[cf]|gif|png|jpeg|webp)|$)/` |
Member
There was a problem hiding this comment.
More than a regexp, I think a list of types would be more convenient to configure externally here: [ "avi", "heif" ]. Is there any scenario where you need to go lower level and tweak the image bit?
| import { REWRITE_HISTORY_COMMAND } from "../extensions/rewritable_history_extension" | ||
|
|
||
| // Common image types from default config.variable_content_types | ||
| const DEFAULT_PREVIEWABLE_PATTERN = /^image(\/(avif|hei[cf]|gif|png|jpeg|webp)|$)/ |
Member
There was a problem hiding this comment.
Should the default be in the presets? in src/config/lexxy.js
| return contentType.match(this.PREVIEWABLE_PATTERN) | ||
| } | ||
|
|
||
| static get PREVIEWABLE_PATTERN() { |
Member
There was a problem hiding this comment.
Minor: a bit unusual to use a constant nomenclature for something that is not a constant. I would use a regular method name previewablePattern
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.
Restrict previewable image types for determining what can be previewed in-browser and expect a preview from the server. Only image corresponding to the types will be galleryable.
Files can still be replaced by a preview when the server sends
previewable: truein the Blob attributes.I'm proposing a default of
/^image(\/(avif|hei[cf]|gif|png|jpeg|webp)|$)/which expands Trix's default of/^image(\/(gif|png|webp|jpe?g)|$)/to include more modern file types from the default ActiveStoragevariable_content_types.I'm unsure to include TIFFs, PSDs or ICOs in the pattern as they don't feel like they belong in a gallery since they can't be viewed directly in the browser. Would we rather complete alignment with the default
variable?file types and figure out the upload preview situation?Full default
config.active_storage.variable_content_typeslists:image/png
image/gif
image/jpeg
image/tiff
image/bmp
image/vnd.adobe.photoshop
image/vnd.microsoft.icon
image/webp
image/avif
image/heic
image/heif