From e89f9d84b513a6a89dec55633fd7a55ed4e7be43 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 27 Nov 2021 21:17:43 +0100 Subject: [PATCH 1/7] make tests more reliable The old tests check if the uploaded SVG was included in the HTML, immediately after setting the HTML. In case Etherpad would remove the image due to some error it's possible that the test returns true even if the image gets removed in the end. To make this a little less likely, prior to checking the HTML, let's wait for an ACCEPT message from the server. --- static/tests/frontend/specs/insert.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/static/tests/frontend/specs/insert.js b/static/tests/frontend/specs/insert.js index b1eb8c6..14ea3a0 100644 --- a/static/tests/frontend/specs/insert.js +++ b/static/tests/frontend/specs/insert.js @@ -10,8 +10,19 @@ describe('Image Upload', function () { it('Puts an image in the pad and ensure it isnt removed', async function () { this.timeout(10000); const inner$ = helper.padInner$; - inner$('div:eq(2)').html('hello world'); + + await helper.edit('hello world\n', 3); + + // clear the first line + inner$('div').first().html('
'); + + // wait for the edit to be accepted + await helper.waitForPromise(() => helper.commits.length === 2); + inner$('div').first().html(``); + + await helper.waitForPromise(() => helper.commits.length === 3); + await helper.waitForPromise(() => inner$('div').first().html().indexOf(uploadSVG) !== -1, 1000); await helper.waitForPromise( () => inner$('div:eq(2)').text().indexOf('hello world') !== -1, 1000); @@ -22,12 +33,16 @@ describe('Image Upload', function () { const inner$ = helper.padInner$; // puts hello world on second line - inner$('div:eq(1)').html('hello world'); + await helper.edit('hello world\n', 2); + await helper.waitForPromise(() => inner$('div:eq(1)').text() === 'hello world', 1000); // puts image on first line inner$('div').first().html(``); + // wait for the edit to be accepted + await helper.waitForPromise(() => helper.commits.length === 2); + await helper.waitForPromise(() => inner$('div').first().html().indexOf(uploadSVG) !== -1, 1000); await helper.waitForPromise( () => inner$('div:eq(1)').text().indexOf('hello world') !== -1, 1000); From 85158a6d77b8174cda0efb5601b31027f640cd15 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 27 Nov 2021 21:27:17 +0100 Subject: [PATCH 2/7] skip tests if storageType is not base64 --- static/tests/frontend/specs/insert.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/static/tests/frontend/specs/insert.js b/static/tests/frontend/specs/insert.js index 14ea3a0..c76e63f 100644 --- a/static/tests/frontend/specs/insert.js +++ b/static/tests/frontend/specs/insert.js @@ -1,13 +1,19 @@ 'use strict'; describe('Image Upload', function () { + let storageType; + // create a new pad before each test run - beforeEach(function (cb) { - helper.newPad(cb); + beforeEach(async function () { + await helper.aNewPad(); + storageType = helper.padChrome$.window.clientVars.ep_image_upload && + helper.padChrome$.window.clientVars.ep_image_upload.storageType; this.timeout(60000); }); it('Puts an image in the pad and ensure it isnt removed', async function () { + if (storageType !== 'base64') this.skip(); + this.timeout(10000); const inner$ = helper.padInner$; @@ -29,6 +35,8 @@ describe('Image Upload', function () { }); it('Puts an image in the pad and next line is not modified', async function () { + if (storageType !== 'base64') this.skip(); + this.timeout(10000); const inner$ = helper.padInner$; From 3c125117db52e887576687492495c1b7f858829e Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 27 Nov 2021 22:00:50 +0100 Subject: [PATCH 3/7] test storagetype local --- static/tests/frontend/specs/insert.js | 99 ++++++++++++++++++++++++--- 1 file changed, 89 insertions(+), 10 deletions(-) diff --git a/static/tests/frontend/specs/insert.js b/static/tests/frontend/specs/insert.js index c76e63f..633970f 100644 --- a/static/tests/frontend/specs/insert.js +++ b/static/tests/frontend/specs/insert.js @@ -1,5 +1,48 @@ 'use strict'; +/** + * @param {number} line the line of the image + * @returns {string} embedded image data + */ +const fetchImageFromLine = async (line) => { + let imageUrl; + const inner$ = helper.padInner$; + + // wait for img tag to appear + await helper.waitForPromise(() => { + const image = inner$(`div:eq(${line})`)[0].querySelector('img'); + if (image) { + imageUrl = image.getAttribute('src'); + if (/^http/.test(imageUrl)) { + return true; + } else { + return false; + } + } + }, 2000); + + // get image + let embeddedImage; + $.ajax({ + type: 'GET', + url: imageUrl, + success: (data) => { + embeddedImage = data; + }, + error: (error) => { + throw error; + }, + cache: false, + processData: false, + dataType: 'text', + }); + + // size of uploadSVG blob + await helper.waitForPromise(() => embeddedImage && embeddedImage.length === 214690); + + return embeddedImage; +}; + describe('Image Upload', function () { let storageType; @@ -12,8 +55,6 @@ describe('Image Upload', function () { }); it('Puts an image in the pad and ensure it isnt removed', async function () { - if (storageType !== 'base64') this.skip(); - this.timeout(10000); const inner$ = helper.padInner$; @@ -25,18 +66,36 @@ describe('Image Upload', function () { // wait for the edit to be accepted await helper.waitForPromise(() => helper.commits.length === 2); + // in case of copy&paste the image is placed at the cursor position + // let's put the cursor onto the first line + const firstLine = inner$('div:eq(0)'); + helper.selectLines(firstLine, firstLine, 1, 1); + inner$('div').first().html(``); await helper.waitForPromise(() => helper.commits.length === 3); - await helper.waitForPromise(() => inner$('div').first().html().indexOf(uploadSVG) !== -1, 1000); - await helper.waitForPromise( - () => inner$('div:eq(2)').text().indexOf('hello world') !== -1, 1000); + if (storageType === 'base64') { + await helper.waitForPromise(() => inner$('div:eq(0)').html().indexOf(uploadSVG) !== -1, 1000); + await helper.waitForPromise( + () => inner$('div:eq(2)').text().indexOf('hello world') !== -1, 1000); + } else { + const image = await fetchImageFromLine(0); + const uploadedSVGData = uploadSVG.match(/^data:([^;]+);base64,(.*)/); + + expect(window.atob(uploadedSVGData[2])).to.be(image); + + // ensure the image is actually displayed + const height = window.getComputedStyle(inner$('div:eq(0)')[0].querySelector('img')).height; + expect(parseInt(height)).to.be.above(200); + + // uploadFile calls `ace.ace_doReturnKey()` so there is an additional newline + await helper.waitForPromise( + () => inner$('div:eq(3)').text().indexOf('hello world') !== -1, 1000); + } }); it('Puts an image in the pad and next line is not modified', async function () { - if (storageType !== 'base64') this.skip(); - this.timeout(10000); const inner$ = helper.padInner$; @@ -45,15 +104,35 @@ describe('Image Upload', function () { await helper.waitForPromise(() => inner$('div:eq(1)').text() === 'hello world', 1000); + // in case of copy&paste the image is placed at the cursor position + // let's put the cursor onto the first line + const firstLine = inner$('div:eq(0)'); + helper.selectLines(firstLine, firstLine, 1, 1); + // puts image on first line inner$('div').first().html(``); // wait for the edit to be accepted await helper.waitForPromise(() => helper.commits.length === 2); - await helper.waitForPromise(() => inner$('div').first().html().indexOf(uploadSVG) !== -1, 1000); - await helper.waitForPromise( - () => inner$('div:eq(1)').text().indexOf('hello world') !== -1, 1000); + if (storageType === 'base64') { + await helper.waitForPromise(() => inner$('div:eq(0)').html().indexOf(uploadSVG) !== -1, 1000); + await helper.waitForPromise( + () => inner$('div:eq(1)').text().indexOf('hello world') !== -1, 1000); + } else { + const image = await fetchImageFromLine(0); + const uploadedSVGData = uploadSVG.match(/^data:([^;]+);base64,(.*)/); + + expect(window.atob(uploadedSVGData[2])).to.be(image); + + // ensure the image is actually displayed + const height = window.getComputedStyle(inner$('div:eq(0)')[0].querySelector('img')).height; + expect(parseInt(height)).to.be.above(200); + + // uploadFile calls `ace.ace_doReturnKey()` so there is an additional newline + await helper.waitForPromise( + () => inner$('div:eq(2)').text().indexOf('hello world') !== -1, 1000); + } }); }); From 4085c0cce4183816963d0e93f2a04a2c794b1d0b Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 27 Nov 2021 22:46:54 +0100 Subject: [PATCH 4/7] move upload logic into a separate function --- static/js/toolbar.js | 89 +++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 43 deletions(-) diff --git a/static/js/toolbar.js b/static/js/toolbar.js index 5b95f92..9a4cdea 100644 --- a/static/js/toolbar.js +++ b/static/js/toolbar.js @@ -47,6 +47,51 @@ const _isValid = (file) => { return true; }; +const uploadFile = (context, file, filename) => { + const formData = new FormData(); + + // add assoc key values, this will be posts values + formData.append('file', file, filename ? filename : file.name); + $('#imageUploadModalLoader').addClass('popup-show'); + $.ajax({ + type: 'POST', + url: `${clientVars.padId}/pluginfw/ep_image_upload/upload`, + xhr: () => { + const myXhr = $.ajaxSettings.xhr(); + + return myXhr; + }, + success: (data) => { + $('#imageUploadModalLoader').removeClass('popup-show'); + context.ace.callWithAce((ace) => { + const imageLineNr = _handleNewLines(ace); + ace.ace_addImage(imageLineNr, data); + ace.ace_doReturnKey(); + }, 'img', true); + }, + error: (error) => { + let errorResponse; + try { + errorResponse = JSON.parse(error.responseText.trim()); + if (errorResponse.type) { + errorResponse.message = window._(`ep_image_upload.error.${errorResponse.type}`); + } + } catch (err) { + errorResponse = {message: error.responseText}; + } + + $('#imageUploadModalLoader').removeClass('popup-show'); + $('#imageUploadModalError .error').html(errorResponse.message); + $('#imageUploadModalError').addClass('popup-show'); + }, + async: true, + data: formData, + cache: false, + contentType: false, + processData: false, + timeout: 60000, + }); +}; exports.postToolbarInit = (hook, context) => { const toolbar = context.toolbar; @@ -83,49 +128,7 @@ exports.postToolbarInit = (hook, context) => { }, 'img', true); }; } else { - const formData = new FormData(); - - // add assoc key values, this will be posts values - formData.append('file', file, file.name); - $('#imageUploadModalLoader').addClass('popup-show'); - $.ajax({ - type: 'POST', - url: `${clientVars.padId}/pluginfw/ep_image_upload/upload`, - xhr: () => { - const myXhr = $.ajaxSettings.xhr(); - - return myXhr; - }, - success: (data) => { - $('#imageUploadModalLoader').removeClass('popup-show'); - context.ace.callWithAce((ace) => { - const imageLineNr = _handleNewLines(ace); - ace.ace_addImage(imageLineNr, data); - ace.ace_doReturnKey(); - }, 'img', true); - }, - error: (error) => { - let errorResponse; - try { - errorResponse = JSON.parse(error.responseText.trim()); - if (errorResponse.type) { - errorResponse.message = window._(`ep_image_upload.error.${errorResponse.type}`); - } - } catch (err) { - errorResponse = {message: error.responseText}; - } - - $('#imageUploadModalLoader').removeClass('popup-show'); - $('#imageUploadModalError .error').html(errorResponse.message); - $('#imageUploadModalError').addClass('popup-show'); - }, - async: true, - data: formData, - cache: false, - contentType: false, - processData: false, - timeout: 60000, - }); + uploadFile(context, file); } }); $(document).find('body').find('#imageInput').trigger('click'); From c7f766d91c3dba8d72e6722cb19724ad1492dce9 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sat, 27 Nov 2021 22:05:27 +0100 Subject: [PATCH 5/7] export _isValid and uploadFile --- static/js/toolbar.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/static/js/toolbar.js b/static/js/toolbar.js index 9a4cdea..3528d36 100644 --- a/static/js/toolbar.js +++ b/static/js/toolbar.js @@ -46,6 +46,7 @@ const _isValid = (file) => { return true; }; +exports._isValid = _isValid; const uploadFile = (context, file, filename) => { const formData = new FormData(); @@ -92,6 +93,7 @@ const uploadFile = (context, file, filename) => { timeout: 60000, }); }; +exports.uploadFile = uploadFile; exports.postToolbarInit = (hook, context) => { const toolbar = context.toolbar; From 38a8a71daf607f69aa16e338a17e85f5465a6121 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sun, 28 Nov 2021 03:46:49 +0100 Subject: [PATCH 6/7] make _isValid return a valid extension if the mimetype is valid --- static/js/toolbar.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/static/js/toolbar.js b/static/js/toolbar.js index 3528d36..35a57c9 100644 --- a/static/js/toolbar.js +++ b/static/js/toolbar.js @@ -16,6 +16,7 @@ const _handleNewLines = (ace) => { const _isValid = (file) => { const mimedb = clientVars.ep_image_upload.mimeTypes; const mimeType = mimedb[file.type]; + const extension = mimeType ? mimeType.extensions[0] : null; let validMime = null; if (clientVars.ep_image_upload && clientVars.ep_image_upload.fileTypes) { validMime = false; @@ -44,7 +45,7 @@ const _isValid = (file) => { return false; } - return true; + return extension; }; exports._isValid = _isValid; From af2b1413760af1d9f5f578adb9b53ee75f8e5dd1 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Sun, 28 Nov 2021 00:26:40 +0100 Subject: [PATCH 7/7] respect storageType on copy&paste Instead of observing the clipboard, we first put the data URI into the pad text, so it is uploaded by contentCollector. When Etherpad starts using the Clipboard API we should use it too. --- static/js/contentCollection.js | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/static/js/contentCollection.js b/static/js/contentCollection.js index 36a555b..d4726e8 100644 --- a/static/js/contentCollection.js +++ b/static/js/contentCollection.js @@ -1,11 +1,13 @@ 'use strict'; +const {_isValid, uploadFile} = require('ep_image_upload/static/js/toolbar'); + // When an image is detected give it a lineAttribute // of Image with the URL to the iamge exports.collectContentImage = (hookName, {node, state: {lineAttributes}, tname}) => { if (tname === 'div' || tname === 'p') delete lineAttributes.img; if (tname !== 'img') return; - lineAttributes.img = + const imageData = // Client-side. This will also be used for server-side HTML imports once jsdom adds support // for HTMLImageElement.currentSrc. node.currentSrc || @@ -13,6 +15,33 @@ exports.collectContentImage = (hookName, {node, state: {lineAttributes}, tname}) node.src || // Server-side HTML imports using cheerio (Etherpad <= v1.8.14). (node.attribs && node.attribs.src); + + if (typeof window !== 'undefined' && clientVars.ep_image_upload.storageType === 'local') { + if (/^http/.test(imageData)) { + // an uploaded image is copied, place a copy in the desired line + lineAttributes.img = imageData; + return; + } + + const padeditor = require('ep_etherpad-lite/static/js/pad_editor').padeditor; + + const match = imageData.match(/data:([^;]+);base64,(.*)/); + if (!match || !match[1] || !match[2]) return; + + // decode from internal base64 rep + const decodedData = Uint8Array.from(window.atob(match[2]), (c) => c.charCodeAt(0)); + + // check if size is within limits and mime type is supported + const extension = _isValid({size: decodedData.length, type: match[1]}); + if (!extension) return; + + const blob = new Blob([decodedData], {type: match[1]}); + + // image.* is a temporary name not used on the server + uploadFile(padeditor, blob, `image.${extension}`); + } else { + lineAttributes.img = imageData; + } }; exports.collectContentPre = (name, context) => {