Skip to content

fix(pdfjs): fix pdf viewer url construction#761

Merged
cristianoliveira merged 1 commit intov4-devfrom
fix/v4-pdfjs-double-encoding
Jan 7, 2026
Merged

fix(pdfjs): fix pdf viewer url construction#761
cristianoliveira merged 1 commit intov4-devfrom
fix/v4-pdfjs-double-encoding

Conversation

@cristianoliveira
Copy link
Copy Markdown
Contributor

This commit changes how the PDF viewer URL is built to avoid double encoding of the file parameter.

Detailed Changes:

  • Implementation:
    • Changed editor.js to use URL object for constructing viewer URL.
    • Changed viewer.mjs to decode file parameter before URL parsing.

This commit changes how the PDF viewer URL is built to avoid double encoding of the file parameter.

Detailed Changes:
 - Implementation:
   - Changed editor.js to use URL object for constructing viewer URL.
   - Changed viewer.mjs to decode file parameter before URL parsing.
Copy link
Copy Markdown
Member

@cdujeu cdujeu left a comment

Choose a reason for hiding this comment

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

lgtm - to be retested on ci deployment

file = params.get("file") ?? AppOptions.get("defaultUrl");
try {
file = new URL(decodeURIComponent(file)).href;
file = new URL(file).href;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small suggestion to protect against file names failing in the new URL(file).href statement

if(file.toString().startsWith('http`) {
 // construct
}

Or similar

Copy link
Copy Markdown
Contributor

@osalama7 osalama7 left a comment

Choose a reason for hiding this comment

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

lgtm

@cristianoliveira cristianoliveira merged commit a5b88ae into v4-dev Jan 7, 2026
2 checks passed
@cristianoliveira cristianoliveira deleted the fix/v4-pdfjs-double-encoding branch January 7, 2026 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants