Skip to content

Fix: markdown download cancellation logic #3348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tyroneyeh wants to merge 2 commits into
base: master
Choose a base branch
from
+3 −0
Open

Fix: markdown download cancellation logic #3348

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e4d1fe2
Fix markdown download cancellation logic
tyroneyeh May 28, 2026
572781d
Fix markdown download interception to only apply to viewer URLs
tyroneyeh May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/ui/main/serverView/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ const initializeServerWebContentsAfterAttach = (
webviewSession.on('will-download', (_event, item) => {
if (item.getFilename().endsWith('.md')) {
const downloadUrl = item.getURL();
// Only intercept for viewer URLs; allow normal downloads to proceed.
// Rocket.Chat server appends 'download=' for view actions vs. download actions.
if (!downloadUrl.endsWith('download=')) return;
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for URL construction patterns related to markdown downloads in the codebase

echo "=== Searching for download URL patterns ==="
rg -n -C5 'download=' --type ts --type tsx --type js --type jsx

echo -e "\n=== Searching for markdown-related download handling ==="
rg -n -C5 '\.md|markdown' --type ts --type tsx | rg -C3 'download|Download'

echo -e "\n=== Searching for document viewer URL construction ==="
rg -n -C5 'DOCUMENT_VIEWER.*URL' --type ts --type tsx

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 335

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/ui/main/serverView/index.ts"

echo "=== Code around line 179 in $FILE ==="
nl -ba "$FILE" | sed -n '150,210p'

echo
echo "=== Search for endsWith('download=') usage ==="
rg -n "endsWith\\('download='\\)" "$FILE" || true

echo
echo "=== Search for downloadUrl variable and its origin in $FILE ==="
rg -n "downloadUrl" "$FILE" -S

echo
echo "=== Search repository for 'download=' query construction patterns (ts/js only) ==="
rg -n -S "download=" --type-add "ts:*.ts" --type ts --type-add "js:*.js" --type js || true

echo
echo "=== Search repository for will-download handler ==="
rg -n -S "will-download" "$FILE" || true

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 200

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/ui/main/serverView/index.ts"

echo "=== Code around line ~179 in $FILE ==="
awk 'NR>=150 && NR<=210 {printf "%d\t%s\n", NR, $0}' "$FILE" | sed -n '1,200p'

echo
echo "=== Search for endsWith('download=') usage in $FILE ==="
rg -n "endsWith\\('download='\\)" "$FILE" || true

echo
echo "=== Search for will-download handler in $FILE ==="
rg -n "will-download" "$FILE" || true

echo
echo "=== Search for downloadUrl variable and its origin in $FILE ==="
rg -n "downloadUrl" "$FILE" || true

echo
echo "=== Search for download query construction patterns (ts/js only) ==="
rg -n -S "download=" --type-add "ts:*.ts" --type-add "js:*.js" --type ts --type js || true

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 2732

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Search repo for 'download=' literal usages (ts/js only) ==="
rg -n -S "download=" --type-add "ts:*.ts" --type-add "js:*.js" --type ts --type js || true

echo
echo "=== Search repo for SERVER_DOCUMENT_VIEWER_OPEN_URL ==="
rg -n "SERVER_DOCUMENT_VIEWER_OPEN_URL" --type-add "ts:*.ts" --type-add "tsx:*.tsx" --type-add "js:*.js" --type-add "jsx:*.jsx" --type ts --type tsx --type js --type jsx || true

echo
echo "=== Search repo for documentFormat: 'markdown' usage ==="
rg -n "documentFormat:\s*'markdown'" --type-add "ts:*.ts" --type-add "tsx:*.tsx" --type-add "js:*.js" --type-add "jsx:*.jsx" --type ts --type tsx --type js --type jsx || true

echo
echo "=== Search repo for 'download' query param parsing ==="
rg -n -S "searchParams\.get\\('download'\\)|searchParams\.has\\('download'\\)|\\bdownload\\b.*(query|string)" --type-add "ts:*.ts" --type-add "tsx:*.tsx" --type-add "js:*.js" --type-add "jsx:*.jsx" --type ts --type tsx --type js --type jsx || true

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 1452

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect small relevant files/sections
sed -n '1,120p' src/servers/actions.ts

echo "----"
sed -n '240,330p' src/servers/reducers.ts

echo "----"
sed -n '1,120p' src/documentViewer/ipc.ts

echo "----"
# show the viewer open handler usage context
sed -n '150,260p' src/ui/components/ServersView/ServerPane.tsx

echo "----"
sed -n '1,80p' src/ui/main/serverView/index.ts

Repository: RocketChat/Rocket.Chat.Electron

Length of output: 9679

Harden the markdown download intercept URL check (line 179)

  • if (!downloadUrl.endsWith('download=')) return; is an exact suffix match; it won’t trigger if download isn’t literally the final characters (e.g., additional query params/values, fragments, or different server URL shape).
  • Replace the suffix logic with robust URL parsing (new URL(item.getURL()) + searchParams) and document what Rocket.Chat generates for the “open in document viewer vs allow download” case.
  • Repo search shows this download= literal is only used here; SERVER_DOCUMENT_VIEWER_OPEN_URL just forwards documentUrl unchanged, so correctness depends entirely on what Electron reports in item.getURL().
🤖 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 `@src/ui/main/serverView/index.ts` at line 179, Replace the brittle suffix
check that tests downloadUrl.endsWith('download=') with URL parsing: call new
URL(item.getURL()), then inspect url.searchParams (e.g., if
(!url.searchParams.has('download')) return; or check a specific value with
url.searchParams.get('download') if Rocket.Chat uses a value like 'true'/'1' or
empty string for the download flag); update the logic around the
downloadUrl/item.getURL() handling and SERVER_DOCUMENT_VIEWER_OPEN_URL usage to
rely on url.searchParams so additional query params or fragments won’t break the
detection.

item.cancel();
dispatch({
type: SERVER_DOCUMENT_VIEWER_OPEN_URL,
Expand Down
Loading