From 0ba242ea4a674aeb5ef0e15248931824fba5da38 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 23 Nov 2022 10:40:30 +0100 Subject: [PATCH 1/2] Support FileAttachments with hash-signs in the filename (issue 15729) The reason for the issue is that we use the generic `getFilenameFromUrl` helper function, which was originally intended for regular URLs. For the filenames we're dealing with in FileAttachments, we really only want to strip the path when one exists[1]. --- [1] See [bug 1230933](https://bugzilla.mozilla.org/show_bug.cgi?id=1230933) for an example of such a case. --- src/display/annotation_layer.js | 2 +- src/display/display_utils.js | 15 ++++++++------- test/unit/display_utils_spec.js | 7 +++++++ web/pdf_attachment_viewer.js | 5 ++++- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index c8a156fc1..84ac665b1 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -2496,7 +2496,7 @@ class FileAttachmentAnnotationElement extends AnnotationElement { super(parameters, { isRenderable: true }); const { filename, content } = this.data.file; - this.filename = getFilenameFromUrl(filename); + this.filename = getFilenameFromUrl(filename, /* onlyStripPath = */ true); this.content = content; this.linkService.eventBus?.dispatch("fileattachmentannotation", { diff --git a/src/display/display_utils.js b/src/display/display_utils.js index 1d04f0efa..f904b122d 100644 --- a/src/display/display_utils.js +++ b/src/display/display_utils.js @@ -334,15 +334,16 @@ function isPdfFile(filename) { /** * Gets the filename from a given URL. * @param {string} url + * @param {boolean} [onlyStripPath] * @returns {string} */ -function getFilenameFromUrl(url) { - const anchor = url.indexOf("#"); - const query = url.indexOf("?"); - const end = Math.min( - anchor > 0 ? anchor : url.length, - query > 0 ? query : url.length - ); +function getFilenameFromUrl(url, onlyStripPath = false) { + let end = url.length; + if (!onlyStripPath) { + const anchor = url.indexOf("#"); + const query = url.indexOf("?"); + end = Math.min(anchor > 0 ? anchor : end, query > 0 ? query : end); + } return url.substring(url.lastIndexOf("/", end) + 1, end); } diff --git a/test/unit/display_utils_spec.js b/test/unit/display_utils_spec.js index 55ebe6e7d..d37399a32 100644 --- a/test/unit/display_utils_spec.js +++ b/test/unit/display_utils_spec.js @@ -190,6 +190,13 @@ describe("display_utils", function () { const url = "https://server.org/filename.pdf?foo=bar"; expect(getFilenameFromUrl(url)).toEqual("filename.pdf"); }); + + it("should get the filename from a relative URL, keeping the anchor", function () { + const url = "../../part1#part2.pdf"; + expect(getFilenameFromUrl(url, /* onlyStripPath = */ true)).toEqual( + "part1#part2.pdf" + ); + }); }); describe("getPdfFilenameFromUrl", function () { diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js index 2bf4bb88a..c138a34b3 100644 --- a/web/pdf_attachment_viewer.js +++ b/web/pdf_attachment_viewer.js @@ -118,7 +118,10 @@ class PDFAttachmentViewer extends BaseTreeViewer { for (const name of names) { const item = attachments[name]; const content = item.content, - filename = getFilenameFromUrl(item.filename); + filename = getFilenameFromUrl( + item.filename, + /* onlyStripPath = */ true + ); const div = document.createElement("div"); div.className = "treeItem"; From f3e0f866412ac7c4d04afef9cadd9b6433e32602 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 23 Nov 2022 11:48:08 +0100 Subject: [PATCH 2/2] Simplify the `getFilenameFromUrl` helper function --- src/display/display_utils.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/display/display_utils.js b/src/display/display_utils.js index f904b122d..f6d245741 100644 --- a/src/display/display_utils.js +++ b/src/display/display_utils.js @@ -338,13 +338,10 @@ function isPdfFile(filename) { * @returns {string} */ function getFilenameFromUrl(url, onlyStripPath = false) { - let end = url.length; if (!onlyStripPath) { - const anchor = url.indexOf("#"); - const query = url.indexOf("?"); - end = Math.min(anchor > 0 ? anchor : end, query > 0 ? query : end); + [url] = url.split(/[#?]/, 1); } - return url.substring(url.lastIndexOf("/", end) + 1, end); + return url.substring(url.lastIndexOf("/") + 1); } /**