github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-06-06 23:07:28 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

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

Browse Source 
* 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
This commit is contained in:
jbrw
2021-06-07 13:03:53 -04:00
committed by GitHub
parent 36e0e6a322
commit 09bc95d46b
5 changed files with 34 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
app/assets/javascripts/discourse/app/lib/utilities.js
View File

@ -160,6 +160,7 @@ export function selectedText() {
range.setEndBefore($postMenuArea); range.setEndBefore($postMenuArea);
} }
const $oneboxTest = $ancestor.closest("aside.onebox[data-onebox-src]");
const $codeBlockTest = $ancestor.parents("pre"); const $codeBlockTest = $ancestor.parents("pre");
if ($codeBlockTest.length) { if ($codeBlockTest.length) {
const $code = $("<code>"); const $code = $("<code>");
@ -172,11 +173,21 @@ export function selectedText() {
} else { } else {
$div.append($code); $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 { } else {
$div.append(range.cloneContents()); $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()); return toMarkdown($div.html());
} }

27
app/assets/javascripts/discourse/tests/acceptance/topic-quote-button-test.js
View File

@ -5,9 +5,9 @@ import {
} from "discourse/tests/helpers/qunit-helpers"; } from "discourse/tests/helpers/qunit-helpers";
import I18n from "I18n"; import I18n from "I18n";
import { test } from "qunit"; 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 range = document.createRange();
const node = document.querySelector(selector); const node = document.querySelector(selector);
range.selectNodeContents(node); range.selectNodeContents(node);
@ -15,6 +15,7 @@ function selectText(selector) {
const selection = window.getSelection(); const selection = window.getSelection();
selection.removeAllRanges(); selection.removeAllRanges();
selection.addRange(range); selection.addRange(range);
await settled();
} }
acceptance("Topic - Quote button - logged in", function (needs) { 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) { test("Does not show the quote share buttons by default", async function (assert) {
await visit("/t/internationalization-localization/280"); 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.ok(exists(".insert-quote"), "it shows the quote button");
assert.equal( assert.equal(
queryAll(".quote-sharing").length, queryAll(".quote-sharing").length,
@ -39,7 +40,7 @@ acceptance("Topic - Quote button - logged in", function (needs) {
this.siteSettings.share_quote_visibility = "all"; this.siteSettings.share_quote_visibility = "all";
await visit("/t/internationalization-localization/280"); 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(exists(".quote-sharing"), "it shows the quote sharing options");
assert.ok( assert.ok(
@ -51,6 +52,18 @@ acceptance("Topic - Quote button - logged in", function (needs) {
"it includes the email share button" "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) { 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) { test("Shows quote share buttons with the right site settings", async function (assert) {
await visit("/t/internationalization-localization/280"); 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(queryAll(".quote-sharing"), "it shows the quote sharing options");
assert.ok( assert.ok(
@ -83,7 +96,7 @@ acceptance("Topic - Quote button - anonymous", function (needs) {
this.siteSettings.share_quote_buttons = "twitter"; this.siteSettings.share_quote_buttons = "twitter";
await visit("/t/internationalization-localization/280"); 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(exists(".quote-sharing"), "it shows the quote sharing options");
assert.ok( assert.ok(
@ -101,7 +114,7 @@ acceptance("Topic - Quote button - anonymous", function (needs) {
this.siteSettings.share_quote_visibility = "none"; this.siteSettings.share_quote_visibility = "none";
await visit("/t/internationalization-localization/280"); await visit("/t/internationalization-localization/280");
selectText("#post_5 blockquote"); await selectText("#post_5 blockquote");
assert.equal( assert.equal(
queryAll(".quote-sharing").length, queryAll(".quote-sharing").length,

2
app/assets/javascripts/discourse/tests/fixtures/topic.js vendored
View File

@ -5455,7 +5455,7 @@ export default {
username: "group_moderator", username: "group_moderator",
avatar_template: "/images/avatar.png", avatar_template: "/images/avatar.png",
created_at: "2020-07-24T17:50:17.274Z", 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_number: 3,
post_type: 1, post_type: 1,
updated_at: "2020-07-24T17:50:17.274Z", updated_at: "2020-07-24T17:50:17.274Z",

2
lib/onebox/templates/_layout.mustache
View File

@ -1,4 +1,4 @@
<aside class="onebox {{subname}}"> <aside class="onebox {{subname}}" data-onebox-src="{{link}}">
<header class="source"> <header class="source">
{{#favicon}} {{#favicon}}
<img src="{{favicon}}" class="site-icon"/> <img src="{{favicon}}" class="site-icon"/>

2
spec/lib/onebox/engine/amazon_onebox_spec.rb
View File

@ -208,7 +208,7 @@ describe Onebox::Engine::AmazonOnebox do
end end
it "produces a placeholder" do 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 end
it "returns errors" do it "returns errors" do