feat: E2E_PDF_Preview_Size_Limit setting for encrypted pdfs preview#40517
Open
nazabucciarelli wants to merge 10 commits into
Open
feat: E2E_PDF_Preview_Size_Limit setting for encrypted pdfs preview#40517nazabucciarelli wants to merge 10 commits into
E2E_PDF_Preview_Size_Limit setting for encrypted pdfs preview#40517nazabucciarelli wants to merge 10 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 7a51828 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughGenericFileAttachment now handles encrypted ChangesEncrypted PDF Viewing on Desktop
Sequence Diagram: sequenceDiagram
participant User
participant GenericFileAttachment
participant RocketChatDesktop
participant ServiceWorker
participant Server
User->>GenericFileAttachment: click attachment link
GenericFileAttachment->>GenericFileAttachment: detect /file-decrypt/ and format PDF
alt Desktop viewer available & under size limit
GenericFileAttachment->>Server: fetch decrypted PDF (abortable)
Server-->>GenericFileAttachment: decrypted blob
GenericFileAttachment->>GenericFileAttachment: create object URL
GenericFileAttachment->>RocketChatDesktop: openDocumentViewer(object URL)
else Over size limit or viewer not used
GenericFileAttachment->>ServiceWorker: registerDownloadForUid / forAttachmentDownload
ServiceWorker-->>User: start download
end
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40517 +/- ##
===========================================
- Coverage 69.63% 69.58% -0.06%
===========================================
Files 3318 3329 +11
Lines 121981 122948 +967
Branches 21813 21920 +107
===========================================
+ Hits 84947 85552 +605
- Misses 33701 34046 +345
- Partials 3333 3350 +17
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates file attachment handling so PDF attachments—especially encrypted PDFs—can be previewed inline and downloaded from the PDF viewer instead of failing through the previous encrypted download path.
Changes:
- Adds a helper to request decrypted attachment bytes from the service worker.
- Routes PDF title clicks through browser/native PDF preview or Electron document viewer.
- Preserves the existing service-worker download path for encrypted non-PDF files.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
apps/meteor/client/hooks/useDownloadFromServiceWorker.ts |
Adds getDecryptedBuffer for retrieving decrypted attachment data from the service worker. |
apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx |
Splits PDF handling into encrypted blob-preview and non-encrypted inline-preview flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nazabucciarelli
force-pushed
the
fix/pdf-viewer-encrypted-room
branch
from
3d105f4 to
77b5b46
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx`:
- Around line 38-45: The cleanup effect currently revokes blobUrlRef.current on
unmount but doesn’t prevent a concurrent fetch from later assigning a new blob
URL, causing a leak; update the component to add a mounted flag (e.g., let
isMounted = true in useEffect and set false on cleanup) and/or use an
AbortController to cancel the in-flight fetch, and only set blobUrlRef.current
(and call URL.createObjectURL) if isMounted is true and the fetch was not
aborted; apply the same guard to the fetch logic referenced around the
download/fetch function (the code that assigns blobUrlRef.current, lines ~66-71)
so any newly created blob URL is only stored while mounted and revoked on
cleanup (and abort the fetch on unmount).
- Around line 63-71: The blob URL leak happens when multiple clicks start
overlapping fetches because only blobUrlRef.current is tracked; fix by
introducing an AbortController stored (e.g., in a ref like fetchAbortRef) and
before starting a new fetch abort any in-flight request and revoke any
previously created blob URL (blobUrlRef.current) to ensure cleanup; then create
a new AbortController for the new fetch, pass its signal to fetch(getURL(link)),
await response.blob(), create the new object URL, set blobUrlRef.current and
openDocumentViewer(blobUrl, format, title ?? ''), and ensure you handle
AbortError (skip revoke if already revoked) and always revoke old blob URLs when
replacing them to prevent leaks.
- Around line 66-71: Wrap the fetch/get blob sequence in a try-catch around the
code that calls fetch(getURL(link)), response.blob(), URL.createObjectURL and
openDocumentViewer so network or blob errors are caught; on success set
blobUrlRef.current and call openDocumentViewer(blobUrl, format, title ?? ''), on
failure log the error (console.error) and surface a user-facing error (e.g.,
show a toast/alert or call a provided notification helper) so clicking gives
feedback; also ensure any created object URL is revoked on error/cleanup (use
URL.revokeObjectURL) so resources aren’t leaked and blobUrlRef.current is only
assigned after successful creation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6ad6eb8c-d3d7-4ff1-a1fa-efdb623d2af3
📒 Files selected for processing (2)
.changeset/fine-jokes-trade.mdapps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
🧠 Learnings (3)
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/fine-jokes-trade.md
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
🔇 Additional comments (1)
.changeset/fine-jokes-trade.md (1)
1-5: LGTM!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx">
<violation number="1" location="apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx:79">
P2: Check `response.ok` before converting to blob; HTTP error responses currently get opened in the PDF viewer as if they were valid files.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/server/settings/e2e.ts (1)
37-42: ⚡ Quick winGate this setting behind encrypted file support.
This limit only affects encrypted file previews, so showing it unconditionally leaves a setting visible even when
E2E_Enable_Encrypt_Filesis off and the value has no effect.Suggested fix
await this.add('E2E_PDF_Preview_Size_Limit', 10, { type: 'int', public: true, + enableQuery: { _id: 'E2E_Enable_Encrypt_Files', value: true }, i18nLabel: 'E2E_PDF_Preview_Size_Limit', i18nDescription: 'E2E_PDF_Preview_Size_Limit_Description', });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/server/settings/e2e.ts` around lines 37 - 42, The setting E2E_PDF_Preview_Size_Limit is being added unconditionally but only applies when encrypted file support is enabled; wrap the this.add call for 'E2E_PDF_Preview_Size_Limit' in a conditional that checks the 'E2E_Enable_Encrypt_Files' flag (e.g. await this.get('E2E_Enable_Encrypt_Files') or equivalent) and only register the setting when that flag is true so it is hidden/ignored when encrypted files are disabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx`:
- Around line 77-91: The fetch uses abortControllerRef.current directly, which
can be replaced by a new controller on overlap and cause stale blob URLs; fix by
creating a request-local const controller = new AbortController(), use
controller.signal for fetch and aborted checks, and after creating blobUrl only
set blobUrlRef.current if abortControllerRef.current === controller (otherwise
revoke the new blobUrl and exit); ensure you still assign
abortControllerRef.current = controller before the fetch and revoke any previous
blobUrl when replacing it.
---
Nitpick comments:
In `@apps/meteor/server/settings/e2e.ts`:
- Around line 37-42: The setting E2E_PDF_Preview_Size_Limit is being added
unconditionally but only applies when encrypted file support is enabled; wrap
the this.add call for 'E2E_PDF_Preview_Size_Limit' in a conditional that checks
the 'E2E_Enable_Encrypt_Files' flag (e.g. await
this.get('E2E_Enable_Encrypt_Files') or equivalent) and only register the
setting when that flag is true so it is hidden/ignored when encrypted files are
disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1f9bb5cf-545d-41b6-9ba0-daf9ec709bf6
📒 Files selected for processing (3)
apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsxapps/meteor/server/settings/e2e.tspackages/i18n/src/locales/en.i18n.json
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/src/locales/en.i18n.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/settings/e2e.tsapps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
🧠 Learnings (4)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/settings/e2e.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/settings/e2e.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/server/settings/e2e.tsapps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
cardoso
reviewed
May 19, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/server/settings/e2e.ts (1)
37-42: ⚡ Quick winConsider adding
enableQueryfor consistency.All other E2E-specific settings in this file include
enableQuery: { _id: 'E2E_Enable', value: true }to ensure they are only active when E2E encryption is enabled. The newE2E_PDF_Preview_Size_Limitsetting lacks this query, making it visible in the admin panel even when E2E is disabled. For consistency and to avoid confusion, consider adding the same enableQuery.♻️ Proposed fix
await this.add('E2E_PDF_Preview_Size_Limit', 10485760, { type: 'int', public: true, i18nLabel: 'E2E_PDF_Preview_Size_Limit', i18nDescription: 'E2E_PDF_Preview_Size_Limit_Description', + enableQuery: { _id: 'E2E_Enable', value: true }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/server/settings/e2e.ts` around lines 37 - 42, The new setting added via this.add('E2E_PDF_Preview_Size_Limit', ...) is missing the enableQuery used by other E2E settings; update the options object passed to the add call (the same object that contains type/public/i18nLabel/i18nDescription) to include enableQuery: { _id: 'E2E_Enable', value: true } so the setting is only visible when E2E is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/i18n/src/locales/en.i18n.json`:
- Line 1925: Update the string value for the i18n key
"E2E_PDF_Preview_Size_Limit_Description" to use correct wording: change "loaded
to memory" to "loaded into memory" so the description reads that encrypted files
are loaded into memory for previews and larger files are downloaded directly.
---
Nitpick comments:
In `@apps/meteor/server/settings/e2e.ts`:
- Around line 37-42: The new setting added via
this.add('E2E_PDF_Preview_Size_Limit', ...) is missing the enableQuery used by
other E2E settings; update the options object passed to the add call (the same
object that contains type/public/i18nLabel/i18nDescription) to include
enableQuery: { _id: 'E2E_Enable', value: true } so the setting is only visible
when E2E is enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29e00358-8742-4329-9c4d-637baf95fdd5
📒 Files selected for processing (3)
apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsxapps/meteor/server/settings/e2e.tspackages/i18n/src/locales/en.i18n.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/settings/e2e.ts
🧠 Learnings (3)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/server/settings/e2e.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/server/settings/e2e.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/server/settings/e2e.ts
nazabucciarelli
changed the title
E2E_PDF_Preview_Size_Limit setting for encrypted pdfs preview
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.
Proposed changes (including videos or screenshots)
I’ll divide the fix in 2 steps:
1- Rocket.Chat Electron Repo: I’ll need to add ‘blob:’ as a supported protocol in src/documentViewer/ipc.ts so that the sent blob is served by the PDF viewer in Electron. This one-line PR was raised for that.
2- Rocket.Chat Main Repo: In the
GenericFileAttachmentcomponent (the one that renders attachments), I've added two different flows for the electron path:PDF size limit for previews (MB)setting value, it won't be converted to blob but downloaded, otherwise it would be very resource consuming. To avoid memory leaks, blob urls are properly revoked.No tests will be implemented since we can't test the PDF viewer from the Electron app.
Issue(s)
SUP-1022 Download fail on Encrypted rooms PDF Viewer
Steps to test or reproduce
Steps to reproduce:
1- Run the Rocket.Chat server in this branch.
2- Pull this branch and run
yarn startto start the desktop project.3- Once in the desktop app, go to Workspace -> Settings -> End-to-end encryption.
4- Enable 'End-to-end encryption' and 'Encrypt files' settings.
5- Notice the new 'PDF size limit for previews (MB)' setting, set a value (e.g 5mb).
6- Upload a .pdf file that exceeds the specified setting and another one under that limit.
7- Click on PDF titles.
8- If the PDF exceeds the setting's value, then it should be directly downloaded, otherwise you should be able to open the PDF preview.
9- Once on the PDF preview, click on the download button from the native embedded PDF preview.
Expected behavior: The encrypted PDF file is successfully downloaded.
Further comments
Summary by CodeRabbit
Bug Fixes
New Features