From aaec80413d756eba1df4769b93adca3053cd82f1 Mon Sep 17 00:00:00 2001 From: zogstrip Date: Tue, 10 Dec 2024 11:49:47 +0100 Subject: [PATCH] FIX: fast edit with a typographic character MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a post containing an apostrophe (') is being cooked, the apostrophe is being converted to the "typographic" version (’) (because we enable markdown-it's **typographer** mode by default in Discourse) When you select text that contains such apostrophe and then try to save your fast edit, it fails miserably without any error. That's because when you select text from the DOM, it uses the cooked version which has the typographic apostrophe. When you save your fast edit, we fetch the raw version of the post, which has the "regular" apostrophe. Thus doing `raw.replace(selectedText, editedText)` doesn't work because `raw` has the regular apostrophe but `selectedText` has the typographic apostrophe. Since it's somewhat complicated to handle all typographic characters, we would basically have to reverse the process done in `custom-typographer-replacements.js`, we instead bail out and show the composer when we detect such character in the selection. Internal ref - t/143836 --- .../discourse/app/components/fast-edit.gjs | 7 ++ .../app/components/post-text-selection.gjs | 1 + .../tests/acceptance/fast-edit-test.js | 17 +++- spec/system/post_selection_fast_edit_spec.rb | 89 ++++++++++--------- 4 files changed, 67 insertions(+), 47 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/fast-edit.gjs b/app/assets/javascripts/discourse/app/components/fast-edit.gjs index c68e8d8e777..d6df5ab7226 100644 --- a/app/assets/javascripts/discourse/app/components/fast-edit.gjs +++ b/app/assets/javascripts/discourse/app/components/fast-edit.gjs @@ -53,6 +53,13 @@ export default class FastEdit extends Component { const result = await ajax(`/posts/${this.args.post.id}`); const newRaw = result.raw.replace(this.args.initialValue, this.value); + // Warn the user if we failed to update the post + if (newRaw === result.raw) { + throw new Error( + "Failed to update the post. Did your fast edit include a special character?" + ); + } + await this.args.post.save({ raw: newRaw }); } catch (error) { popupAjaxError(error); diff --git a/app/assets/javascripts/discourse/app/components/post-text-selection.gjs b/app/assets/javascripts/discourse/app/components/post-text-selection.gjs index f92e55c7f0e..af9a9b51969 100644 --- a/app/assets/javascripts/discourse/app/components/post-text-selection.gjs +++ b/app/assets/javascripts/discourse/app/components/post-text-selection.gjs @@ -190,6 +190,7 @@ export default class PostTextSelection extends Component { quoteState.buffer.length === 0 || quoteState.buffer.includes("|") || // tables are too complex quoteState.buffer.match(/\n/g) || // linebreaks are too complex + quoteState.buffer.match(/[‚‘’„“”«»‹›™±…→←↔¶]/g) || // typopgraphic characters are too complex matches?.length !== 1 // duplicates are too complex ) { supportsFastEdit = false; diff --git a/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js b/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js index cd8b91c2b39..43743d16d7f 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/fast-edit-test.js @@ -11,9 +11,7 @@ import { cloneJSON } from "discourse-common/lib/object"; acceptance("Fast Edit", function (needs) { needs.user(); - needs.settings({ - enable_fast_edit: true, - }); + needs.settings({ enable_fast_edit: true }); needs.pretender((server, helper) => { server.get("/posts/419", () => { return helper.response(cloneJSON(postFixtures["/posts/398"])); @@ -86,6 +84,19 @@ acceptance("Fast Edit", function (needs) { assert.dom(".d-editor-input").exists(); }); + test("Opens full composer when selection has typographic characters", async function (assert) { + await visit("/t/internationalization-localization/280"); + + query("#post_2 .cooked").append(`That’s what she said!`); + 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(); + }); + test("Works with diacritics", async function (assert) { await visit("/t/internationalization-localization/280"); diff --git a/spec/system/post_selection_fast_edit_spec.rb b/spec/system/post_selection_fast_edit_spec.rb index fe03dd4d8fe..ee6763abc39 100644 --- a/spec/system/post_selection_fast_edit_spec.rb +++ b/spec/system/post_selection_fast_edit_spec.rb @@ -3,46 +3,54 @@ describe "Post selection | Fast edit", type: :system do let(:topic_page) { PageObjects::Pages::Topic.new } let(:fast_editor) { PageObjects::Components::FastEditor.new } + fab!(:topic) - fab!(:post) { Fabricate(:post, topic: topic) } - fab!(:post_2) { Fabricate(:post, topic: topic, raw: "It ‘twas a great’ “time”!") } - fab!(:spanish_post) { Fabricate(:post, topic: topic, raw: "Hola Juan, ¿cómo estás?") } - fab!(:chinese_post) { Fabricate(:post, topic: topic, raw: "这是一个测试") } - fab!(:post_with_emoji) { Fabricate(:post, topic: topic, raw: "Good morning :wave:!") } + + fab!(:post) { Fabricate(:post, topic:) } + fab!(:post_2) { Fabricate(:post, topic:, raw: "It ‘twas a great’ “time”!") } + fab!(:spanish_post) { Fabricate(:post, topic:, raw: "Hola Juan, ¿cómo estás?") } + fab!(:chinese_post) { Fabricate(:post, topic:, raw: "这是一个测试") } + fab!(:post_with_emoji) { Fabricate(:post, topic:, raw: "Good morning :wave:!") } fab!(:post_with_quote) do Fabricate( :post, - topic: topic, + topic:, raw: "[quote]\n#{post_2.raw}\n[/quote]\n\nBelle journée, n'est-ce pas ?", ) end + fab!(:current_user) { Fabricate(:admin) } before { sign_in(current_user) } - context "when text selected it opens contact menu and fast editor" do - it "opens context menu and fast edit dialog" do + def css(post) = "#{topic_page.post_by_number_selector(post.post_number)} .cooked p" + + context "when text is selected" do + before do topic_page.visit_topic(topic) - - select_text_range("#{topic_page.post_by_number_selector(1)} .cooked p", 0, 10) - expect(topic_page.fast_edit_button).to be_visible - - topic_page.click_fast_edit_button - expect(topic_page.fast_edit_input).to be_visible + select_text_range(css(post), 0, 5) end - it "edits first paragraph and saves changes" do - topic_page.visit_topic(topic) + it "opens context menu" do + expect(topic_page.fast_edit_button).to be_visible + end - select_text_range("#{topic_page.post_by_number_selector(1)} .cooked p", 0, 5) - topic_page.click_fast_edit_button + context "when clicking the fast edit button" do + before { topic_page.click_fast_edit_button } - fast_editor.fill_content("Howdy") - fast_editor.save + it "opens the fast editor" do + expect(topic_page.fast_edit_input).to be_visible + end - within("#post_1 .cooked > p") do |el| - expect(el).not_to eq("Hello world") - expect(el).to have_content("Howdy") + context "when entering some text and clicking the save button" do + before do + fast_editor.fill_content("Howdy") + fast_editor.save + end + + it "saves changes" do + expect(page).to have_selector(css(post), text: "Howdy world") + end end end end @@ -51,7 +59,7 @@ describe "Post selection | Fast edit", type: :system do it "opens the composer directly" do topic_page.visit_topic(topic) - select_text_range("#{topic_page.post_by_number_selector(6)} .cooked p", 5, 10) + select_text_range(css(post_with_quote), 5, 10) topic_page.click_fast_edit_button expect(topic_page).to have_expanded_composer @@ -59,63 +67,56 @@ describe "Post selection | Fast edit", type: :system do end context "when editing text that has strange characters" do - it "saves when paragraph contains apostrophe" do + it "saves when paragraph contains apostrophes" do topic_page.visit_topic(topic) - select_text_range("#{topic_page.post_by_number_selector(2)} .cooked p", 19, 4) + select_text_range(css(post_2), 19, 4) topic_page.click_fast_edit_button fast_editor.fill_content("day") fast_editor.save - expect(page).to have_selector( - "#{topic_page.post_by_number_selector(2)} .cooked p", - text: "It ‘twas a great’ “day”!", - ) + expect(page).to have_selector(css(post_2), text: "It ‘twas a great’ “day”!") end - it "saves when text contains diacratics" do + it "saves when text contains diacritics" do topic_page.visit_topic(topic) - select_text_range("#{topic_page.post_by_number_selector(3)} .cooked p", 11, 12) + select_text_range(css(spanish_post), 11, 12) topic_page.click_fast_edit_button fast_editor.fill_content("¿está todo bien?") fast_editor.save - expect(page).to have_selector( - "#{topic_page.post_by_number_selector(3)} .cooked p", - text: "Hola Juan, ¿está todo bien?", - ) + expect(page).to have_selector(css(spanish_post), text: "Hola Juan, ¿está todo bien?") end it "saves when text contains CJK ranges" do topic_page.visit_topic(topic) - select_text_range("#{topic_page.post_by_number_selector(4)} .cooked p", 0, 2) + select_text_range(css(chinese_post), 0, 2) topic_page.click_fast_edit_button fast_editor.fill_content("今天") fast_editor.save - expect(page).to have_selector( - "#{topic_page.post_by_number_selector(4)} .cooked p", - text: "今天一个测试", - ) + expect(page).to have_selector(css(chinese_post), text: "今天一个测试") end it "saves when text contains emoji" do topic_page.visit_topic(topic) - select_text_range("#{topic_page.post_by_number_selector(5)} .cooked p", 5, 7) + select_text_range(css(post_with_emoji), 5, 7) topic_page.click_fast_edit_button fast_editor.fill_content("day") fast_editor.save - expect(page).to have_no_css("#fast-edit-input") - expect(post_with_emoji.raw).to eq("Good day :wave:!") + # NOTE: the emoji isn't picked up by the "text:" selector + expect(page).to have_selector(css(post_with_emoji), text: "Good day !") + # So we also check the raw content to ensure it's been saved correctly + expect(post_with_emoji.reload.raw).to eq "Good day :wave:!" end end end