Skip to content

Commit 1f7ac1b

Browse files
SamSaffronSamSaffron
authored
FIX: maintain extension when quoting images (#36240)
Also do not show quote image to anonymous
1 parent f5aac07 commit 1f7ac1b

File tree

6 files changed

+112
-45
lines changed

6 files changed

+112
-45
lines changed

‎frontend/discourse/app/lib/lightbox.js‎

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import { isRailsTesting, isTesting } from "discourse/lib/environment";
55
import { helperContext } from "discourse/lib/helpers";
66
import { renderIcon } from "discourse/lib/icon-library";
77
import { SELECTORS } from "discourse/lib/lightbox/constants";
8-
import quoteImage, { canQuoteImage } from "discourse/lib/lightbox/quote-image";
8+
import quoteImage, {
9+
canBuildImageQuote,
10+
} from "discourse/lib/lightbox/quote-image";
911
import { isDocumentRTL } from "discourse/lib/text-direction";
1012
import {
1113
escapeExpression,
1214
postRNWebviewMessage,
1315
} from "discourse/lib/utilities";
14-
import User from "discourse/models/user";
1516
import { i18n } from "discourse-i18n";
1617

1718
export async function loadMagnificPopup() {
@@ -27,10 +28,12 @@ export default async function lightbox(
2728
return;
2829
}
2930

31+
const currentUser = helperContext()?.currentUser;
3032
const caps = helperContext().capabilities;
3133
const imageClickNavigation = caps.touch;
3234
const canDownload =
33-
!siteSettings.prevent_anons_from_downloading_files || User.current();
35+
!siteSettings.prevent_anons_from_downloading_files || !!currentUser;
36+
const canQuoteImage = !!currentUser;
3437

3538
if (siteSettings.experimental_lightbox) {
3639
const { default: PhotoSwipeLightbox } = await import("photoswipe/lightbox");
@@ -204,37 +207,39 @@ export default async function lightbox(
204207
},
205208
});
206209

207-
lightboxEl.pswp.ui.registerElement({
208-
name: "quote-image",
209-
order: 10,
210-
isButton: true,
211-
title: i18n("lightbox.quote"),
212-
html: {
213-
isCustomSVG: true,
214-
inner:
215-
'<path id="pswp__icn-quote" d="M0 216C0 149.7 53.7 96 120 96l8 0c17.7 0 32 14.3 32 32s-14.3 32-32 32l-8 0c-30.9 0-56 25.1-56 56l0 8 64 0c35.3 0 64 28.7 64 64l0 64c0 35.3-28.7 64-64 64l-64 0c-35.3 0-64-28.7-64-64l0-32 0-32 0-72zm256 0c0-66.3 53.7-120 120-120l8 0c17.7 0 32 14.3 32 32s-14.3 32-32 32l-8 0c-30.9 0-56 25.1-56 56l0 8 64 0c35.3 0 64 28.7 64 64l0 64c0 35.3-28.7 64-64 64l-64 0c-35.3 0-64-28.7-64-64l0-32 0-32 0-72z"/>',
216-
outlineID: "pswp__icn-quote",
217-
size: 512,
218-
},
219-
onInit: (el, pswp) => {
220-
pswp.on("change", () => {
221-
const slideData = pswp.currSlide?.data;
210+
if (canQuoteImage) {
211+
lightboxEl.pswp.ui.registerElement({
212+
name: "quote-image",
213+
order: 10,
214+
isButton: true,
215+
title: i18n("lightbox.quote"),
216+
html: {
217+
isCustomSVG: true,
218+
inner:
219+
'<path id="pswp__icn-quote" d="M0 216C0 149.7 53.7 96 120 96l8 0c17.7 0 32 14.3 32 32s-14.3 32-32 32l-8 0c-30.9 0-56 25.1-56 56l0 8 64 0c35.3 0 64 28.7 64 64l0 64c0 35.3-28.7 64-64 64l-64 0c-35.3 0-64-28.7-64-64l0-32 0-32 0-72zm256 0c0-66.3 53.7-120 120-120l8 0c17.7 0 32 14.3 32 32s-14.3 32-32 32l-8 0c-30.9 0-56 25.1-56 56l0 8 64 0c35.3 0 64 28.7 64 64l0 64c0 35.3-28.7 64-64 64l-64 0c-35.3 0-64-28.7-64-64l0-32 0-32 0-72z"/>',
220+
outlineID: "pswp__icn-quote",
221+
size: 512,
222+
},
223+
onInit: (el, pswp) => {
224+
pswp.on("change", () => {
225+
const slideData = pswp.currSlide?.data;
226+
const slideElement = slideData?.element;
227+
el.style.display = canBuildImageQuote(slideElement, slideData)
228+
? ""
229+
: "none";
230+
});
231+
},
232+
onClick: () => {
233+
const slideData = lightboxEl.pswp.currSlide?.data;
222234
const slideElement = slideData?.element;
223-
el.style.display = canQuoteImage(slideElement, slideData)
224-
? ""
225-
: "none";
226-
});
227-
},
228-
onClick: () => {
229-
const slideData = lightboxEl.pswp.currSlide?.data;
230-
const slideElement = slideData?.element;
231-
quoteImage(slideElement, slideData).then((didQuote) => {
232-
if (didQuote) {
233-
lightboxEl.pswp.close();
234-
}
235-
});
236-
},
237-
});
235+
quoteImage(slideElement, slideData).then((didQuote) => {
236+
if (didQuote) {
237+
lightboxEl.pswp.close();
238+
}
239+
});
240+
},
241+
});
242+
}
238243

239244
lightboxEl.pswp.ui.registerElement({
240245
name: "custom-counter",

‎frontend/discourse/app/lib/lightbox/quote-image.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { getOwner } from "@ember/owner";
22
import { helperContext } from "discourse/lib/helpers";
3-
import { buildImageMarkdown as buildImageMarkdownShared } from "discourse/lib/markdown-image-builder";
3+
import {
4+
buildImageMarkdown as buildImageMarkdownShared,
5+
extensionFromUrl,
6+
} from "discourse/lib/markdown-image-builder";
47
import { buildQuote } from "discourse/lib/quote";
58
import Composer from "discourse/models/composer";
69
import Draft from "discourse/models/draft";
@@ -16,7 +19,11 @@ function buildImageMarkdown(slideElement, slideData) {
1619

1720
// Check for base62 SHA1 to use short upload:// URL format (same as to-markdown.js)
1821
if (slideData.base62SHA1) {
22+
const extension = extensionFromUrl(slideData.src);
1923
src = `upload://${slideData.base62SHA1}`;
24+
if (extension) {
25+
src += `.${extension}`;
26+
}
2027
} else {
2128
// Prefer data-orig-src (same as to-markdown.js)
2229
src = slideData.origSrc || slideData.src;
@@ -35,7 +42,14 @@ function buildImageMarkdown(slideElement, slideData) {
3542
});
3643
}
3744

38-
export function canQuoteImage(slideElement, slideData) {
45+
/**
46+
* Checks if the slide element and data can be used to build a quote.
47+
*
48+
* @param {Element} slideElement
49+
* @param {object} slideData
50+
* @returns {boolean}
51+
*/
52+
export function canBuildImageQuote(slideElement, slideData) {
3953
return buildImageMarkdown(slideElement, slideData) !== null;
4054
}
4155

‎frontend/discourse/app/lib/markdown-image-builder.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,22 @@ export function sanitizeAlt(text, options = {}) {
1313
return trimmed.replace(/\|/g, "&#124;").replace(/([\\\[\]])/g, "\\$1");
1414
}
1515

16+
/**
17+
* Extracts the extension (without dot) from a URL or path.
18+
* Returns null when no extension is present.
19+
*
20+
* @param {string} url
21+
* @returns {string|null}
22+
*/
23+
export function extensionFromUrl(url) {
24+
if (!url) {
25+
return null;
26+
}
27+
28+
const match = url.match(/\.([a-zA-Z0-9]+)(?:\?|$)/);
29+
return match ? match[1] : null;
30+
}
31+
1632
export function buildImageMarkdown(imageData) {
1733
const {
1834
src,

‎frontend/discourse/app/lib/to-markdown.js‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import { buildImageMarkdown } from "discourse/lib/markdown-image-builder";
1+
import {
2+
buildImageMarkdown,
3+
extensionFromUrl,
4+
} from "discourse/lib/markdown-image-builder";
25

36
const MSO_LIST_CLASSES = [
47
"MsoListParagraphCxSpFirst",
@@ -365,6 +368,14 @@ export class Tag {
365368

366369
if (base62SHA1) {
367370
href = `upload://${base62SHA1}`;
371+
const extension =
372+
extensionFromUrl(img.attributes.src) ||
373+
extensionFromUrl(attr.href) ||
374+
extensionFromUrl(attr["data-download-href"]);
375+
376+
if (extension) {
377+
href += `.${extension}`;
378+
}
368379
}
369380

370381
const width = img.attributes.width;
@@ -413,6 +424,13 @@ export class Tag {
413424
const base62SHA1 = attr["data-base62-sha1"];
414425
if (base62SHA1) {
415426
src = `upload://${base62SHA1}`;
427+
const extension =
428+
extensionFromUrl(attr.src || pAttr.src) ||
429+
extensionFromUrl(attr["data-orig-src"]);
430+
431+
if (extension) {
432+
src += `.${extension}`;
433+
}
416434
}
417435

418436
if (cssClass?.includes("emoji")) {

‎frontend/discourse/tests/unit/lib/lightbox/quote-image-test.js‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@ import Service from "@ember/service";
33
import { setupTest } from "ember-qunit";
44
import { module, test } from "qunit";
55
import sinon from "sinon";
6-
import quoteImage, { canQuoteImage } from "discourse/lib/lightbox/quote-image";
6+
import { createHelperContext, helperContext } from "discourse/lib/helpers";
7+
import quoteImage, {
8+
canBuildImageQuote,
9+
} from "discourse/lib/lightbox/quote-image";
710
import Draft from "discourse/models/draft";
11+
import { logIn } from "discourse/tests/helpers/qunit-helpers";
812

913
class ComposerStub extends Service {
1014
model = { viewOpen: false };
@@ -27,6 +31,15 @@ module("Unit | Lib | lightbox | quote image", function (hooks) {
2731
setupTest(hooks);
2832

2933
hooks.beforeEach(function () {
34+
this.originalHelperContext = helperContext();
35+
36+
logIn(this.owner);
37+
38+
createHelperContext({
39+
...(this.originalHelperContext || {}),
40+
currentUser: this.owner.lookup("service:current-user"),
41+
});
42+
3043
this.owner.unregister("service:composer");
3144
this.owner.unregister("service:app-events");
3245

@@ -55,6 +68,7 @@ module("Unit | Lib | lightbox | quote image", function (hooks) {
5568
hooks.afterEach(function () {
5669
document.querySelectorAll(".topic-post").forEach((el) => el.remove());
5770
this.draftGetStub.restore();
71+
createHelperContext(this.originalHelperContext);
5872
});
5973

6074
function buildLightbox(context, overrides = {}) {
@@ -132,12 +146,12 @@ module("Unit | Lib | lightbox | quote image", function (hooks) {
132146
);
133147
});
134148

135-
test("canQuoteImage only returns true when context and metadata exist", function (assert) {
149+
test("canBuildImageQuote only returns true when context and metadata exist", function (assert) {
136150
const invalid = document.createElement("a");
137-
assert.false(canQuoteImage(invalid, {}));
151+
assert.false(canBuildImageQuote(invalid, {}));
138152

139153
const { element, slideData } = buildLightbox(this);
140-
assert.true(canQuoteImage(element, slideData));
154+
assert.true(canBuildImageQuote(element, slideData));
141155
});
142156

143157
test("builds markdown using data-orig-src and dimensions when composer is closed", async function (assert) {
@@ -187,7 +201,7 @@ module("Unit | Lib | lightbox | quote image", function (hooks) {
187201
);
188202
});
189203

190-
test("uses short upload:// URL when data-base62-sha1 is present", async function (assert) {
204+
test("uses short upload:// URL with extension when data-base62-sha1 is present", async function (assert) {
191205
const { element, slideData } = buildLightbox(this, {
192206
base62SHA1: "a4bcwvmLAy8cGHKPUrK4G3AUbt9",
193207
href: "//localhost:4200/uploads/default/original/1X/468eb8aa1f0126f1ce7e7ea7a2f64f25da0b58db.png",
@@ -200,9 +214,9 @@ module("Unit | Lib | lightbox | quote image", function (hooks) {
200214
const quote = this.composer.openCalls[0].quote;
201215
assert.true(
202216
quote.includes(
203-
"![diagram|640x480](upload://a4bcwvmLAy8cGHKPUrK4G3AUbt9)"
217+
"![diagram|640x480](upload://a4bcwvmLAy8cGHKPUrK4G3AUbt9.png)"
204218
),
205-
"uses short upload:// URL format with base62-sha1"
219+
"uses short upload:// URL format with base62-sha1 and extension"
206220
);
207221
});
208222

‎frontend/discourse/tests/unit/lib/to-markdown-test.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ module("Unit | Utility | to-markdown", function (hooks) {
189189
html = `<img src="${url}" width="100" height="50" title="some title" data-base62-sha1="${base62SHA1}">`;
190190
assert.strictEqual(
191191
toMarkdown(html),
192-
`![|100x50](upload://${base62SHA1} "some title")`
192+
`![|100x50](upload://${base62SHA1}.png "some title")`
193193
);
194194

195195
html = `<div><span><img src="${url}" alt="description" width="50" height="100" /></span></div>`;
@@ -437,7 +437,7 @@ helloWorld();</code>consectetur.`;
437437
<span class="filename">sherlock3_sig.jpg</span><span class="informations">5496×3664 2 MB</span><span class="expand"></span>
438438
</div></a>
439439
`;
440-
markdown = `![sherlock3_sig.jpg|689x459](upload://1frsimI7TOtFJyD2LLyKSHM8JWe)`;
440+
markdown = `![sherlock3_sig.jpg|689x459](upload://1frsimI7TOtFJyD2LLyKSHM8JWe.jpeg)`;
441441

442442
assert.strictEqual(toMarkdown(html), markdown);
443443
});

0 commit comments

Comments
 (0)