Skip to content

Commit

Permalink
FIX: Table pasting issues with uppy (#15787) (#15812)
Browse files Browse the repository at this point in the history
When changing to uppy for file uploads we forgot to add
these conditions to the paste event from 9c96511

Basically, if you are pasting more than just a file (e.g. text,
html, rtf), then we should not handle the file and upload it, and
instead just paste in the text. This causes issues with spreadsheet
tools, that will copy the text representation and also an image
representation of cells to the user's clipboard.

This also moves the paste event for composer-upload-uppy to the
element found by the `editorClass` property, so it shares the paste
event with d-editor (via TextareaTextManipulation), which makes testing
this possible as the ember paste bindings are not picked up unless both
paste events are on the same element.
  • Loading branch information
martin-brennan committed Feb 4, 2022
1 parent be72ae8 commit 7dd9dd8
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 13 deletions.
Expand Up @@ -100,6 +100,7 @@ export function cleanUpComposerUploadMarkdownResolver() {
export default Component.extend(ComposerUploadUppy, {
classNameBindings: ["showToolbar:toolbar-visible", ":wmd-controls"],

editorClass: ".d-editor",
fileUploadElementId: "file-uploader",
mobileFileUploaderId: "mobile-file-upload",
eventPrefix: "composer",
Expand Down
Expand Up @@ -64,7 +64,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
this.fileInputEventListener
);

this.element.removeEventListener("paste", this.pasteEventListener);
this.editorEl?.removeEventListener("paste", this.pasteEventListener);

this.appEvents.off(`${this.eventPrefix}:add-files`, this._addFiles);
this.appEvents.off(
Expand Down Expand Up @@ -92,6 +92,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
this.set("inProgressUploads", []);
this.placeholders = {};
this._preProcessorStatus = {};
this.editorEl = this.element.querySelector(this.editorClass);
this.fileInputEl = document.getElementById(this.fileUploadElementId);
const isPrivateMessage = this.get("composerModel.privateMessage");

Expand All @@ -106,7 +107,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
this.fileInputEl,
this._addFiles
);
this.element.addEventListener("paste", this.pasteEventListener);
this.editorEl.addEventListener("paste", this.pasteEventListener);

this._uppyInstance = new Uppy({
id: this.uppyId,
Expand Down Expand Up @@ -520,12 +521,12 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
return;
}

const { canUpload } = clipboardHelpers(event, {
const { canUpload, canPasteHtml, types } = clipboardHelpers(event, {
siteSettings: this.siteSettings,
canUpload: true,
});

if (!canUpload) {
if (!canUpload || canPasteHtml || types.includes("text/plain")) {
return;
}

Expand Down
Expand Up @@ -2,12 +2,13 @@ import {
acceptance,
createFile,
loggedInUser,
paste,
query,
} from "discourse/tests/helpers/qunit-helpers";
import { withPluginApi } from "discourse/lib/plugin-api";
import bootbox from "bootbox";
import { authorizedExtensions } from "discourse/lib/uploads";
import { click, fillIn, visit } from "@ember/test-helpers";
import { click, fillIn, settled, visit } from "@ember/test-helpers";
import I18n from "I18n";
import { skip, test } from "qunit";

Expand Down Expand Up @@ -52,6 +53,7 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
needs.pretender(pretender);
needs.settings({
simultaneous_uploads: 2,
enable_rich_text_paste: true,
});

test("should insert the Uploading placeholder then the complete image placeholder", async function (assert) {
Expand Down Expand Up @@ -313,6 +315,40 @@ acceptance("Uppy Composer Attachment - Upload Placeholder", function (needs) {
const image = createFile("avatar.png");
appEvents.trigger("composer:add-files", image);
});

test("should be able to paste a table with files and not upload the files", async function (assert) {
await visit("/");
await click("#create-topic");
const appEvents = loggedInUser().appEvents;
const done = assert.async();

let uppyEventFired = false;

appEvents.on("composer:upload-started", () => {
uppyEventFired = true;
});

let element = query(".d-editor");
let inputElement = query(".d-editor-input");
inputElement.focus();
await paste(element, "\ta\tb\n1\t2\t3", {
types: ["text/plain", "Files"],
files: [createFile("avatar.png")],
});
await settled();

assert.strictEqual(
inputElement.value,
"||a|b|\n|---|---|---|\n|1|2|3|\n",
"only the plain text table is pasted"
);
assert.strictEqual(
uppyEventFired,
false,
"uppy does not start uploading the file"
);
done();
});
});

acceptance("Uppy Composer Attachment - Upload Error", function (needs) {
Expand Down
@@ -1,4 +1,5 @@
import QUnit, { module, skip, test } from "qunit";
import { deepMerge } from "discourse-common/lib/object";
import MessageBus from "message-bus-client";
import {
clearCache as clearOutletCache,
Expand Down Expand Up @@ -561,3 +562,11 @@ export function createFile(name, type = "image/png", blobData = null) {
});
return file;
}

export async function paste(element, text, otherClipboardData = {}) {
let e = new Event("paste", { cancelable: true });
e.clipboardData = deepMerge({ getData: () => text }, otherClipboardData);
element.dispatchEvent(e);
await settled();
return e;
}
Expand Up @@ -5,6 +5,7 @@ import componentTest, {
import {
discourseModule,
exists,
paste,
query,
queryAll,
} from "discourse/tests/helpers/qunit-helpers";
Expand Down Expand Up @@ -822,14 +823,6 @@ third line`
}
);

async function paste(element, text) {
let e = new Event("paste", { cancelable: true });
e.clipboardData = { getData: () => text };
element.dispatchEvent(e);
await settled();
return e;
}

componentTest("paste table", {
template: hbs`{{d-editor value=value composerEvents=true}}`,
beforeEach() {
Expand Down

1 comment on commit 7dd9dd8

@discoursebot
Copy link

Choose a reason for hiding this comment

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

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/bug-with-chat-on-stable/218241/1

Please sign in to comment.