FIX: Quoting Oneboxed content should exclude formatting (#13296)

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
diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js
index 9210d06..d644ffc 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 = $("<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 912c5a6..a72cfec 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 70d5b2f..4196457 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: "<p>Thank you for your reply!</p>",
+          cooked: '<aside class="onebox allowlistedgeneric" data-onebox-src="https://example.com/57350945"><header class="source"><a href="https://example.com/57350945" target="_blank" rel="noopener">XYZ News Site</a> </header> <article class="onebox-body"> <div class="aspect-image" style="--aspect-ratio:690/388;"><img src="/assets/logo.png" class="thumbnail d-lazyload" width="690" height="388"></div> <h3><a href="https://example.com/57350945" target="_blank" rel="noopener">News Headline</a></h3> <p>Article summary</p> </article> <div class="onebox-metadata"> </div> <div style="clear: both"></div> </aside>',
           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 9075319..0288482 100644
--- a/lib/onebox/templates/_layout.mustache
+++ b/lib/onebox/templates/_layout.mustache
@@ -1,4 +1,4 @@
-<aside class="onebox {{subname}}">
+<aside class="onebox {{subname}}" data-onebox-src="{{link}}">
   <header class="source">
     {{#favicon}}
       <img src="{{favicon}}" class="site-icon"/>
diff --git a/spec/lib/onebox/engine/amazon_onebox_spec.rb b/spec/lib/onebox/engine/amazon_onebox_spec.rb
index 7d5973b..9cf409b 100644
--- a/spec/lib/onebox/engine/amazon_onebox_spec.rb
+++ b/spec/lib/onebox/engine/amazon_onebox_spec.rb
@@ -208,7 +208,7 @@ describe Onebox::Engine::AmazonOnebox do
     end
 
     it "produces a placeholder" do
-      expect(onebox.placeholder_html).to include('<aside class="onebox amazon">')
+      expect(onebox.placeholder_html).to include('<aside class="onebox amazon" data-onebox-src="https://www.amazon.com/dp/B0123ABCD3210">')
     end
 
     it "returns errors" do

GitHub sha: 09bc95d4

This commit appears in #13296 which was approved by pmusaraj. It was merged by jbrw.