fix

Author: wxiaoguang

web_src/js/features/comp/EditorUpload.js

@@ -1,11 +1,10 @@
 import {htmlEscape} from 'escape-goat';
 import {POST} from '../../modules/fetch.js';
 import {imageInfo} from '../../utils/image.js';
-import {replaceTextareaSelection} from '../../utils/dom.js';
+import {createElementFromAttrs, replaceTextareaSelection} from '../../utils/dom.js';
 import {isUrl} from '../../utils/url.js';
-import {extname} from '../../utils.js';
+import {extname, isWellKnownImageFilename} from '../../utils.js';
 import {triggerEditorContentChanged} from './EditorMarkdown.js';
-import {getComboMarkdownEditor} from './ComboMarkdownEditor.js';
 
 async function uploadFile(file, uploadUrl) {
   const formData = new FormData();
@@ -81,43 +80,51 @@ class CodeMirrorEditor {
   }
 }
 
-// FIXME: handle non-image files
-async function handleClipboardImages(editor, dropzone, images, e) {
+function isImageFile(file) {
+  return file.type?.startsWith('image/') || isWellKnownImageFilename(file.name);
+}
+
+async function handleUploadFiles(editor, dropzone, files, e) {
   const uploadUrl = dropzone.getAttribute('data-upload-url');
   const filesContainer = dropzone.querySelector('.files');
 
   e.preventDefault();
   e.stopPropagation();
 
-  for (const img of images) {
-    const name = img.name.slice(0, img.name.lastIndexOf('.'));
-    const {width, dppx} = await imageInfo(img);
-    const placeholder = `![${name}](uploading ...)`;
+  for (const file of files) {
+    const name = file.name.slice(0, file.name.lastIndexOf('.'));
+    const isImage = isImageFile(file);
+
+    let placeholder = `[${name}](uploading ...)`;
+    if (isImage) placeholder = `!${placeholder}`;
 
     editor.insertPlaceholder(placeholder);
-    const {uuid} = await uploadFile(img, uploadUrl);
-
-    let text;
-    if (width > 0 && dppx > 1) {
-      // Scale down images from HiDPI monitors. This uses the <img> tag because it's the only
-      // method to change image size in Markdown that is supported by all implementations.
-      // Make the image link relative to the repo path, then the final URL is "/sub-path/owner/repo/attachments/{uuid}"
-      const url = `attachments/${uuid}`;
-      text = `<img width="${Math.round(width / dppx)}" alt="${htmlEscape(name)}" src="${htmlEscape(url)}">`;
+    const {uuid} = await uploadFile(file, uploadUrl);
+
+    let fileMarkdownLink;
+    if (isImage) {
+      const {width, dppx} = await imageInfo(file);
+      if (width > 0 && dppx > 1) {
+        // Scale down images from HiDPI monitors. This uses the <img> tag because it's the only
+        // method to change image size in Markdown that is supported by all implementations.
+        // Make the image link relative to the repo path, then the final URL is "/sub-path/owner/repo/attachments/{uuid}"
+        const url = `attachments/${uuid}`;
+        fileMarkdownLink = `<img width="${Math.round(width / dppx)}" alt="${htmlEscape(name)}" src="${htmlEscape(url)}">`;
+      } else {
+        // Markdown always renders the image with a relative path, so the final URL is "/sub-path/owner/repo/attachments/{uuid}"
+        // TODO: it should also use relative path for consistency, because absolute is ambiguous for "/sub-path/attachments" or "/attachments"
+        const url = `/attachments/${uuid}`;
+        fileMarkdownLink = `![${name}](${url})`;
+      }
     } else {
-      // Markdown always renders the image with a relative path, so the final URL is "/sub-path/owner/repo/attachments/{uuid}"
-      // TODO: it should also use relative path for consistency, because absolute is ambiguous for "/sub-path/attachments" or "/attachments"
       const url = `/attachments/${uuid}`;
-      text = `![${name}](${url})`;
+      fileMarkdownLink = `[${name}](${url})`;
     }
-    editor.replacePlaceholder(placeholder, text);
+    editor.replacePlaceholder(placeholder, fileMarkdownLink);
 
-    const input = document.createElement('input');
-    input.setAttribute('name', 'files');
-    input.setAttribute('type', 'hidden');
-    input.setAttribute('id', uuid);
-    input.value = uuid;
+    const input = createElementFromAttrs('input', {type: 'hidden', name: 'files', id: `dropzone-file-${uuid}`, value: uuid});
     filesContainer.append(input);
+    // TODO: add upload preview item
   }
 }
 
@@ -152,7 +159,7 @@ export function initEasyMDEPaste(easyMDE, dropzone) {
   easyMDE.codemirror.on('paste', (_, e) => {
     const {images} = getPastedContent(e);
     if (images.length) {
-      handleClipboardImages(new CodeMirrorEditor(easyMDE.codemirror), dropzone, images, e);
+      handleUploadFiles(new CodeMirrorEditor(easyMDE.codemirror), dropzone, images, e);
     }
   });
 }
@@ -168,22 +175,22 @@ export function initTextareaPaste(textarea, dropzone) {
   textarea.addEventListener('paste', (e) => {
     const {images, text} = getPastedContent(e);
     if (images.length) {
-      handleClipboardImages(new TextareaEditor(textarea), dropzone, images, e);
+      handleUploadFiles(new TextareaEditor(textarea), dropzone, images, e);
     } else if (text) {
       handleClipboardText(textarea, e, {text, isShiftDown});
     }
   });
   textarea.addEventListener('drop', (e) => {
-    const acceptedFiles = getComboMarkdownEditor(textarea).dropzone.getAttribute('data-accepts');
+    const acceptedFiles = dropzone.getAttribute('data-accepts');
     const files = [];
-    for (const item of e.dataTransfer?.items ?? []) {
+    for (const item of e.dataTransfer.items) {
       if (item?.kind !== 'file') continue;
       const file = item.getAsFile();
+      // FIXME: the check is not right. acceptedFiles could be ".zip, audio/*, image/png, */*"
       if (acceptedFiles.includes(extname(file.name))) {
         files.push(file);
       }
     }
-    // FIXME: handle upload files
-    handleClipboardImages(new TextareaEditor(textarea), dropzone, files, e);
+    handleUploadFiles(new TextareaEditor(textarea), dropzone, files, e);
   });
 }

web_src/js/features/dropzone.js

@@ -78,6 +78,9 @@ export async function initDropzone(dropzoneEl) {
 
   dzInst.on('removedfile', async (file) => {
     if (disableRemovedfileEvent) return;
+
+    // TODO: remove the link from editor
+
     document.querySelector(`#dropzone-file-${file.uuid}`)?.remove();
     // when the uploaded file number reaches the limit, there is no uuid in the dict, and it doesn't need to be removed from server
     if (removeAttachmentUrl && fileUuidDict[file.uuid] && !fileUuidDict[file.uuid].submitted) {

web_src/js/utils.js

@@ -8,7 +8,9 @@ export function basename(path = '') {
 
 // transform /path/to/file.ext to .ext
 export function extname(path = '') {
+  const lastSlashIndex = path.lastIndexOf('/');
   const lastPointIndex = path.lastIndexOf('.');
+  if (lastSlashIndex > lastPointIndex) return '';
   return lastPointIndex < 0 ? '' : path.substring(lastPointIndex);
 }
 
@@ -142,3 +144,7 @@ export function serializeXml(node) {
 }
 
 export const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
+
+export function isWellKnownImageFilename(fn) {
+  return /\.(jpe?g|png|gif|webp|svg)$/i.test(fn);
+}

web_src/js/utils.test.js

@@ -1,7 +1,7 @@
 import {
   basename, extname, isObject, stripTags, parseIssueHref,
   parseUrl, translateMonth, translateDay, blobToDataURI,
-  toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64,
+  toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isWellKnownImageFilename,
 } from './utils.js';
 
 test('basename', () => {
@@ -15,6 +15,7 @@ test('extname', () => {
   expect(extname('/path/')).toEqual('');
   expect(extname('/path')).toEqual('');
   expect(extname('file.js')).toEqual('.js');
+  expect(extname('/my.path/file')).toEqual('');
 });
 
 test('isObject', () => {
@@ -112,3 +113,12 @@ test('encodeURLEncodedBase64, decodeURLEncodedBase64', () => {
   expect(Array.from(decodeURLEncodedBase64('YQ'))).toEqual(Array.from(uint8array('a')));
   expect(Array.from(decodeURLEncodedBase64('YQ=='))).toEqual(Array.from(uint8array('a')));
 });
+
+test('isWellKnownImageFilename', () => {
+  for (const filename of ['a.jpg', '/a.jpeg', '.file.png', '.webp', 'file.svg']) {
+    expect(isWellKnownImageFilename(filename)).toBeTruthy();
+  }
+  for (const filename of ['', 'a.jpg.x', '/path.png/x', 'webp']) {
+    expect(isWellKnownImageFilename(filename)).toBeFalsy();
+  }
+});

web_src/js/utils/dom.js

@@ -1,5 +1,4 @@
 import {debounce} from 'throttle-debounce';
-import {extname} from '../utils.js';
 
 function elementsCall(el, func, ...args) {
   if (typeof el === 'string' || el instanceof String) {