From 09bc95d46b36afb9ce97f87e17b34bc1c50a873d Mon Sep 17 00:00:00 2001 From: jbrw Date: Mon, 7 Jun 2021 13:03:53 -0400 Subject: [PATCH] FIX: Quoting Oneboxed content should exclude formatting (#13296) * FIX: Quoting Oneboxed content should exclude formatting When a post is quoted that includes Oneboxed content, we should not include the formatting generated by the Onebox. Rather, we should attempt to collapse the link referenced by the Onebox to a single line text link. * DEV: fix tests --- .../discourse/app/lib/utilities.js | 11 ++++++++ .../acceptance/topic-quote-button-test.js | 27 ++++++++++++++----- .../discourse/tests/fixtures/topic.js | 2 +- lib/onebox/templates/_layout.mustache | 2 +- spec/lib/onebox/engine/amazon_onebox_spec.rb | 2 +- 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js index 9210d0607e0..d644ffc7d59 100644 --- a/app/assets/javascripts/discourse/app/lib/utilities.js +++ b/app/assets/javascripts/discourse/app/lib/utilities.js @@ -160,6 +160,7 @@ export function selectedText() { range.setEndBefore($postMenuArea); } + const $oneboxTest = $ancestor.closest("aside.onebox[data-onebox-src]"); const $codeBlockTest = $ancestor.parents("pre"); if ($codeBlockTest.length) { const $code = $(""); @@ -172,11 +173,21 @@ export function selectedText() { } else { $div.append($code); } + } else if ($oneboxTest.length) { + // This is a partial quote from a onebox. + // Treat it as though the entire onebox was quoted. + const oneboxUrl = $($oneboxTest).data("onebox-src"); + $div.append(oneboxUrl); } else { $div.append(range.cloneContents()); } } + $div.find("aside.onebox[data-onebox-src]").each(function () { + const oneboxUrl = $(this).data("onebox-src"); + $(this).replaceWith(oneboxUrl); + }); + return toMarkdown($div.html()); } diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js index 912c5a61359..a72cfecdfdf 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js @@ -5,9 +5,9 @@ import { } from "discourse/tests/helpers/qunit-helpers"; import I18n from "I18n"; import { test } from "qunit"; -import { visit } from "@ember/test-helpers"; +import { settled, visit } from "@ember/test-helpers"; -function selectText(selector) { +async function selectText(selector) { const range = document.createRange(); const node = document.querySelector(selector); range.selectNodeContents(node); @@ -15,6 +15,7 @@ function selectText(selector) { const selection = window.getSelection(); selection.removeAllRanges(); selection.addRange(range); + await settled(); } acceptance("Topic - Quote button - logged in", function (needs) { @@ -26,7 +27,7 @@ acceptance("Topic - Quote button - logged in", function (needs) { test("Does not show the quote share buttons by default", async function (assert) { await visit("/t/internationalization-localization/280"); - selectText("#post_5 blockquote"); + await selectText("#post_5 blockquote"); assert.ok(exists(".insert-quote"), "it shows the quote button"); assert.equal( queryAll(".quote-sharing").length, @@ -39,7 +40,7 @@ acceptance("Topic - Quote button - logged in", function (needs) { this.siteSettings.share_quote_visibility = "all"; await visit("/t/internationalization-localization/280"); - selectText("#post_5 blockquote"); + await selectText("#post_5 blockquote"); assert.ok(exists(".quote-sharing"), "it shows the quote sharing options"); assert.ok( @@ -51,6 +52,18 @@ acceptance("Topic - Quote button - logged in", function (needs) { "it includes the email share button" ); }); + + test("Quoting a Onebox should not copy the formatting of the rendered Onebox", async function (assert) { + await visit("/t/topic-for-group-moderators/2480"); + await selectText("#post_3 aside.onebox p"); + await click(".insert-quote"); + + assert.equal( + queryAll(".d-editor-input").val().trim(), + '[quote="group_moderator, post:3, topic:2480"]\nhttps://example.com/57350945\n[/quote]', + "quote only contains a link" + ); + }); }); acceptance("Topic - Quote button - anonymous", function (needs) { @@ -61,7 +74,7 @@ acceptance("Topic - Quote button - anonymous", function (needs) { test("Shows quote share buttons with the right site settings", async function (assert) { await visit("/t/internationalization-localization/280"); - selectText("#post_5 blockquote"); + await selectText("#post_5 blockquote"); assert.ok(queryAll(".quote-sharing"), "it shows the quote sharing options"); assert.ok( @@ -83,7 +96,7 @@ acceptance("Topic - Quote button - anonymous", function (needs) { this.siteSettings.share_quote_buttons = "twitter"; await visit("/t/internationalization-localization/280"); - selectText("#post_5 blockquote"); + await selectText("#post_5 blockquote"); assert.ok(exists(".quote-sharing"), "it shows the quote sharing options"); assert.ok( @@ -101,7 +114,7 @@ acceptance("Topic - Quote button - anonymous", function (needs) { this.siteSettings.share_quote_visibility = "none"; await visit("/t/internationalization-localization/280"); - selectText("#post_5 blockquote"); + await selectText("#post_5 blockquote"); assert.equal( queryAll(".quote-sharing").length, diff --git a/app/assets/javascripts/discourse/tests/fixtures/topic.js b/app/assets/javascripts/discourse/tests/fixtures/topic.js index 70d5b2ff5c4..4196457e6b0 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/topic.js +++ b/app/assets/javascripts/discourse/tests/fixtures/topic.js @@ -5455,7 +5455,7 @@ export default { username: "group_moderator", avatar_template: "/images/avatar.png", created_at: "2020-07-24T17:50:17.274Z", - cooked: "

Thank you for your reply!

", + cooked: '', post_number: 3, post_type: 1, updated_at: "2020-07-24T17:50:17.274Z", diff --git a/lib/onebox/templates/_layout.mustache b/lib/onebox/templates/_layout.mustache index 9075319f891..028848221f5 100644 --- a/lib/onebox/templates/_layout.mustache +++ b/lib/onebox/templates/_layout.mustache @@ -1,4 +1,4 @@ -