Skip to content

Commit

Permalink
FIX: fallback to composer for non ascii characters (#21465)
Browse files Browse the repository at this point in the history
The problem
When selecting text and clicking the "Edit" button that pops up, this opens up the Fast Edit dialog.

The fast edit feature doesn't work well with non standard characters (non-ascii). If the user selects a string of text that contains non-ascii characters, sometimes they won't save. It is non-obvious to the user why this is happening. This issue occurs more frequently when editing content that is written in non-english languages, as fast-edit doesn't work well with non-ascii characters. We currently do a global replace on a couple of the more obvious quotation marks when the fast edit dialog attempts to save, but there are too many edge cases for foreign language content.

The solution
We can fix this issue by using a catch-all approach for non-ascii characters before the user clicks the edit button to bring up the fast edit dialog. Then we can fallback to the full composer to edit their text, which has much better support for non-ascii characters.

What does this regex do?
The regex used matches any character that is not within the ASCII range of 0x00 to 0x7F, which includes all control characters and non-ASCII characters.

This regex pattern can be used to match any character that is not a standard ASCII character, such as accented characters, non-Latin characters, and special symbols.
  • Loading branch information
dbattersby committed May 10, 2023
1 parent 41bdf8c commit 5d8632d
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 8 deletions.
Expand Up @@ -165,12 +165,14 @@ export default Component.extend(KeyEnterEscape, {
if (this._canEditPost) {
const regexp = new RegExp(escapeRegExp(quoteState.buffer), "gi");
const matches = cooked.innerHTML.match(regexp);
const non_ascii_regex = /[^\x00-\x7F]/;

if (
quoteState.buffer.length < 1 ||
quoteState.buffer.includes("|") || // tables are too complex
quoteState.buffer.match(/\n/g) || // linebreaks are too complex
matches?.length > 1 // duplicates are too complex
matches?.length > 1 || // duplicates are too complex
non_ascii_regex.test(quoteState.buffer) // non-ascii chars break fast-edit
) {
this.set("_isFastEditable", false);
this.set("_fastEditInitialSelection", null);
Expand Down
@@ -1,6 +1,5 @@
import {
acceptance,
exists,
query,
selectText,
} from "discourse/tests/helpers/qunit-helpers";
Expand Down Expand Up @@ -28,7 +27,7 @@ acceptance("Fast Edit", function (needs) {
await selectText(textNode, 9);
await click(".quote-button .quote-edit-label");

assert.ok(exists("#fast-edit-input"), "fast editor is open");
assert.dom("#fast-edit-input").exists();
assert.strictEqual(
query("#fast-edit-input").value,
"Any plans",
Expand All @@ -38,7 +37,7 @@ acceptance("Fast Edit", function (needs) {
await fillIn("#fast-edit-input", "My edit");
await click(".save-fast-edit");

assert.notOk(exists("#fast-edit-input"), "fast editor is closed");
assert.dom("#fast-edit-input").doesNotExist();
});

test("Works with keyboard shortcut", async function (assert) {
Expand All @@ -49,7 +48,7 @@ acceptance("Fast Edit", function (needs) {
await selectText(textNode, 9);
await triggerKeyEvent(document, "keypress", "E");

assert.ok(exists("#fast-edit-input"), "fast editor is open");
assert.dom("#fast-edit-input").exists();
assert.strictEqual(
query("#fast-edit-input").value,
"Any plans",
Expand All @@ -59,7 +58,7 @@ acceptance("Fast Edit", function (needs) {
await fillIn("#fast-edit-input", "My edit");
await click(".save-fast-edit");

assert.notOk(exists("#fast-edit-input"), "fast editor is closed");
assert.dom("#fast-edit-input").doesNotExist();
});

test("Opens full composer for multi-line selection", async function (assert) {
Expand All @@ -70,7 +69,22 @@ acceptance("Fast Edit", function (needs) {
await selectText(textNode);
await click(".quote-button .quote-edit-label");

assert.notOk(exists("#fast-edit-input"), "fast editor is not open");
assert.ok(exists(".d-editor-input"), "the composer is open");
assert.dom("#fast-edit-input").doesNotExist();
assert.dom(".d-editor-input").exists();
});

test("Opens full composer when editing non-ascii characters", async function (assert) {
await visit("/t/internationalization-localization/280");

query("#post_2 .cooked").append(
`Je suis désolé, ”comment ça va”? A bientôt!`
);
const textNode = query("#post_2 .cooked").childNodes[2];

await selectText(textNode);
await click(".quote-button .quote-edit-label");

assert.dom("#fast-edit-input").doesNotExist();
assert.dom(".d-editor-input").exists();
});
});

1 comment on commit 5d8632d

@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/fast-edit-doesnt-work-on-content-with-certain-characters/223145/44

Please sign in to comment.