diff --git a/app/assets/javascripts/discourse/components/composer-editor.js.es6 b/app/assets/javascripts/discourse/components/composer-editor.js.es6
index 5195bdb0c73..8417e9579cc 100644
--- a/app/assets/javascripts/discourse/components/composer-editor.js.es6
+++ b/app/assets/javascripts/discourse/components/composer-editor.js.es6
@@ -17,7 +17,7 @@ import {
fetchUnseenTagHashtags
} from "discourse/lib/link-tag-hashtag";
import Composer from "discourse/models/composer";
-import { load } from "pretty-text/oneboxer";
+import { load, LOADING_ONEBOX_CSS_CLASS } from "pretty-text/oneboxer";
import { applyInlineOneboxes } from "pretty-text/inline-oneboxer";
import { ajax } from "discourse/lib/ajax";
import InputValidation from "discourse/models/input-validation";
@@ -37,7 +37,10 @@ import {
resolveAllShortUrls
} from "pretty-text/image-short-url";
-import { INLINE_ONEBOX_LOADING_CSS_CLASS } from "pretty-text/inline-oneboxer";
+import {
+ INLINE_ONEBOX_LOADING_CSS_CLASS,
+ INLINE_ONEBOX_CSS_CLASS
+} from "pretty-text/inline-oneboxer";
const REBUILD_SCROLL_MAP_EVENTS = ["composer:resized", "composer:typed-reply"];
@@ -513,7 +516,7 @@ export default Ember.Component.extend({
applyInlineOneboxes(inline, ajax);
},
- _loadOneboxes($oneboxes) {
+ _loadOneboxes(oneboxes) {
const post = this.get("composer.post");
let refresh = false;
@@ -523,15 +526,19 @@ export default Ember.Component.extend({
post.set("refreshedPost", true);
}
- $oneboxes.each((_, o) =>
- load({
- elem: o,
- refresh,
- ajax,
- categoryId: this.get("composer.category.id"),
- topicId: this.get("composer.topic.id")
- })
- );
+ Object.keys(oneboxes).forEach(oneboxURL => {
+ const onebox = oneboxes[oneboxURL];
+
+ onebox.forEach($onebox => {
+ load({
+ elem: $onebox,
+ refresh,
+ ajax,
+ categoryId: this.get("composer.category.id"),
+ topicId: this.get("composer.topic.id")
+ });
+ });
+ });
},
_warnMentionedGroups($preview) {
@@ -986,31 +993,58 @@ export default Ember.Component.extend({
}
// Paint oneboxes
- const $oneboxes = $("a.onebox", $preview);
- if (
- $oneboxes.length > 0 &&
- $oneboxes.length <= this.siteSettings.max_oneboxes_per_post
- ) {
- Ember.run.debounce(this, this._loadOneboxes, $oneboxes, 450);
- }
+ Ember.run.debounce(
+ this,
+ () => {
+ const inlineOneboxes = {};
+ const oneboxes = {};
+ let oneboxLeft =
+ this.siteSettings.max_oneboxes_per_post -
+ $(
+ `aside.onebox, a.${INLINE_ONEBOX_CSS_CLASS}, a.${LOADING_ONEBOX_CSS_CLASS}`
+ ).length;
+
+ $preview
+ .find(`a.${INLINE_ONEBOX_LOADING_CSS_CLASS}, a.onebox`)
+ .each((_index, link) => {
+ const $link = $(link);
+ const text = $link.text();
+
+ const isInline =
+ $link.attr("class") === INLINE_ONEBOX_LOADING_CSS_CLASS;
+
+ const map = isInline ? inlineOneboxes : oneboxes;
+
+ if (oneboxLeft <= 0) {
+ if (map[text] !== undefined) {
+ map[text].push(link);
+ } else if (isInline) {
+ $link.removeClass(INLINE_ONEBOX_LOADING_CSS_CLASS);
+ }
+ } else {
+ if (!map[text]) {
+ map[text] = [];
+ oneboxLeft--;
+ }
+
+ map[text].push(link);
+ }
+ });
+
+ if (Object.keys(oneboxes).length > 0) {
+ this._loadOneboxes(oneboxes);
+ }
+
+ if (Object.keys(inlineOneboxes).length > 0) {
+ this._loadInlineOneboxes(inlineOneboxes);
+ }
+ },
+ 450
+ );
// Short upload urls need resolution
resolveAllShortUrls(ajax);
- let inline = {};
- $(`a.${INLINE_ONEBOX_LOADING_CSS_CLASS}`, $preview).each(
- (index, link) => {
- const $link = $(link);
- $link.removeClass(INLINE_ONEBOX_LOADING_CSS_CLASS);
- const text = $link.text();
- inline[text] = inline[text] || [];
- inline[text].push($link);
- }
- );
- if (Object.keys(inline).length > 0) {
- Ember.run.debounce(this, this._loadInlineOneboxes, inline, 450);
- }
-
if (this._enableAdvancedEditorPreviewSync()) {
this._syncScroll(
this._syncEditorAndPreviewScroll,
diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6
index f43798897a4..c367f87ed3b 100644
--- a/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6
+++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown/onebox.js.es6
@@ -2,7 +2,8 @@ import { lookupCache } from "pretty-text/oneboxer";
import {
cachedInlineOnebox,
- INLINE_ONEBOX_LOADING_CSS_CLASS
+ INLINE_ONEBOX_LOADING_CSS_CLASS,
+ INLINE_ONEBOX_CSS_CLASS
} from "pretty-text/inline-oneboxer";
const ONEBOX = 1;
@@ -103,6 +104,7 @@ function applyOnebox(state, silent) {
if (onebox && onebox.title) {
text.content = onebox.title;
+ attrs.push(["class", INLINE_ONEBOX_CSS_CLASS]);
} else {
attrs.push(["class", INLINE_ONEBOX_LOADING_CSS_CLASS]);
}
diff --git a/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6.erb b/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6.erb
index 983549dc4e0..9f66c03e742 100644
--- a/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6.erb
+++ b/app/assets/javascripts/pretty-text/inline-oneboxer.js.es6.erb
@@ -3,6 +3,9 @@ let _cache = {};
export const INLINE_ONEBOX_LOADING_CSS_CLASS =
"<%= CookedPostProcessor::INLINE_ONEBOX_LOADING_CSS_CLASS %>";
+export const INLINE_ONEBOX_CSS_CLASS =
+ "<%= CookedPostProcessor::INLINE_ONEBOX_CSS_CLASS %>";
+
export function applyInlineOneboxes(inline, ajax) {
Object.keys(inline).forEach(url => {
// cache a blank locally, so we never trigger a lookup
@@ -17,7 +20,9 @@ export function applyInlineOneboxes(inline, ajax) {
_cache[onebox.url] = onebox;
let links = inline[onebox.url] || [];
links.forEach(link => {
- link.text(onebox.title);
+ $(link).text(onebox.title)
+ .addClass(INLINE_ONEBOX_CSS_CLASS)
+ .removeClass(INLINE_ONEBOX_LOADING_CSS_CLASS);
});
}
});
diff --git a/app/assets/javascripts/pretty-text/oneboxer.js.es6 b/app/assets/javascripts/pretty-text/oneboxer.js.es6
index a7bd2b3ab33..4753ec80158 100644
--- a/app/assets/javascripts/pretty-text/oneboxer.js.es6
+++ b/app/assets/javascripts/pretty-text/oneboxer.js.es6
@@ -1,7 +1,15 @@
let timeout;
const loadingQueue = [];
-const localCache = {};
-const failedCache = {};
+let localCache = {};
+let failedCache = {};
+
+export const LOADING_ONEBOX_CSS_CLASS = "loading-onebox";
+
+export function resetCache() {
+ loadingQueue.clear();
+ localCache = {};
+ failedCache = {};
+}
function resolveSize(img) {
$(img).addClass("size-resolved");
@@ -76,7 +84,7 @@ function loadNext(ajax) {
.finally(() => {
timeout = Ember.run.later(() => loadNext(ajax), timeoutMs);
if (removeLoading) {
- $elem.removeClass("loading-onebox");
+ $elem.removeClass(LOADING_ONEBOX_CSS_CLASS);
$elem.data("onebox-loaded");
}
});
@@ -96,7 +104,7 @@ export function load({
// If the onebox has loaded or is loading, return
if ($elem.data("onebox-loaded")) return;
- if ($elem.hasClass("loading-onebox")) return;
+ if ($elem.hasClass(LOADING_ONEBOX_CSS_CLASS)) return;
const url = elem.href;
@@ -112,7 +120,7 @@ export function load({
}
// Add the loading CSS class
- $elem.addClass("loading-onebox");
+ $elem.addClass(LOADING_ONEBOX_CSS_CLASS);
// Add to the loading queue
loadingQueue.push({ url, refresh, $elem, categoryId, topicId });
diff --git a/app/assets/javascripts/pretty-text/white-lister.js.es6 b/app/assets/javascripts/pretty-text/white-lister.js.es6
index 4e1ed92f01c..c303283e360 100644
--- a/app/assets/javascripts/pretty-text/white-lister.js.es6
+++ b/app/assets/javascripts/pretty-text/white-lister.js.es6
@@ -1,4 +1,7 @@
-import { INLINE_ONEBOX_LOADING_CSS_CLASS } from "pretty-text/inline-oneboxer";
+import {
+ INLINE_ONEBOX_CSS_CLASS,
+ INLINE_ONEBOX_LOADING_CSS_CLASS
+} from "pretty-text/inline-oneboxer";
// to match:
// abcd
@@ -118,6 +121,7 @@ const DEFAULT_LIST = [
"a.mention",
"a.mention-group",
"a.onebox",
+ `a.${INLINE_ONEBOX_CSS_CLASS}`,
`a.${INLINE_ONEBOX_LOADING_CSS_CLASS}`,
"a[data-bbcode]",
"a[name]",
diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index f503149f151..8e8199ca430 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -8,6 +8,9 @@ require_dependency 'quote_comparer'
class CookedPostProcessor
include ActionView::Helpers::NumberHelper
+ INLINE_ONEBOX_LOADING_CSS_CLASS = "inline-onebox-loading"
+ INLINE_ONEBOX_CSS_CLASS = "inline-onebox"
+
attr_reader :cooking_options, :doc
def initialize(post, opts = {})
@@ -30,7 +33,6 @@ class CookedPostProcessor
DistributedMutex.synchronize("post_process_#{@post.id}") do
DiscourseEvent.trigger(:before_post_process_cooked, @doc, @post)
post_process_oneboxes
- post_process_inline_oneboxes
post_process_images
post_process_quotes
optimize_urls
@@ -435,13 +437,35 @@ class CookedPostProcessor
end
def post_process_oneboxes
- Oneboxer.apply(@doc) do |url|
- @has_oneboxes = true
- Oneboxer.onebox(url,
- invalidate_oneboxes: !!@opts[:invalidate_oneboxes],
- user_id: @post&.user_id,
- category_id: @post&.topic&.category_id
- )
+ limit = SiteSetting.max_oneboxes_per_post
+ oneboxes = {}
+ inlineOneboxes = {}
+
+ Oneboxer.apply(@doc, extra_paths: [".#{INLINE_ONEBOX_LOADING_CSS_CLASS}"]) do |url, element|
+ is_onebox = element["class"] == Oneboxer::ONEBOX_CSS_CLASS
+ map = is_onebox ? oneboxes : inlineOneboxes
+ skip_onebox = limit <= 0 && !map[url]
+
+ if skip_onebox
+ remove_inline_onebox_loading_class(element) unless is_onebox
+ next
+ end
+
+ limit -= 1
+ map[url] = true
+
+ if is_onebox
+ @has_oneboxes = true
+
+ Oneboxer.onebox(url,
+ invalidate_oneboxes: !!@opts[:invalidate_oneboxes],
+ user_id: @post&.user_id,
+ category_id: @post&.topic&.category_id
+ )
+ else
+ process_inline_onebox(element)
+ false
+ end
end
oneboxed_images.each do |img|
@@ -577,23 +601,24 @@ class CookedPostProcessor
@doc.try(:to_html)
end
- INLINE_ONEBOX_LOADING_CSS_CLASS = "inline-onebox-loading"
-
private
- def post_process_inline_oneboxes
- @doc.css(".#{INLINE_ONEBOX_LOADING_CSS_CLASS}").each do |element|
- inline_onebox = InlineOneboxer.lookup(
- element.attributes["href"].value,
- invalidate: !!@opts[:invalidate_oneboxes]
- )
+ def process_inline_onebox(element)
+ inline_onebox = InlineOneboxer.lookup(
+ element.attributes["href"].value,
+ invalidate: !!@opts[:invalidate_oneboxes]
+ )
- if title = inline_onebox&.dig(:title)
- element.children = title
- end
-
- element.remove_class(INLINE_ONEBOX_LOADING_CSS_CLASS)
+ if title = inline_onebox&.dig(:title)
+ element.children = title
+ element.add_class(INLINE_ONEBOX_CSS_CLASS)
end
+
+ remove_inline_onebox_loading_class(element)
+ end
+
+ def remove_inline_onebox_loading_class(element)
+ element.remove_class(INLINE_ONEBOX_LOADING_CSS_CLASS)
end
def is_svg?(img)
diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb
index db0a4da32ca..f2781dd5a59 100644
--- a/lib/oneboxer.rb
+++ b/lib/oneboxer.rb
@@ -5,6 +5,8 @@ require_dependency 'final_destination'
Dir["#{Rails.root}/lib/onebox/engine/*_onebox.rb"].sort.each { |f| require f }
module Oneboxer
+ ONEBOX_CSS_CLASS = "onebox"
+
# keep reloaders happy
unless defined? Oneboxer::Result
Result = Struct.new(:doc, :changed) do
@@ -67,11 +69,11 @@ module Oneboxer
end
# Parse URLs out of HTML, returning the document when finished.
- def self.each_onebox_link(string_or_doc)
+ def self.each_onebox_link(string_or_doc, extra_paths: [])
doc = string_or_doc
doc = Nokogiri::HTML::fragment(doc) if doc.is_a?(String)
- onebox_links = doc.search("a.onebox")
+ onebox_links = doc.css("a.#{ONEBOX_CSS_CLASS}", *extra_paths)
if onebox_links.present?
onebox_links.each do |link|
yield(link['href'], link) if link['href'].present?
@@ -83,13 +85,14 @@ module Oneboxer
HTML5_BLOCK_ELEMENTS ||= %w{address article aside blockquote canvas center dd div dl dt fieldset figcaption figure footer form h1 h2 h3 h4 h5 h6 header hgroup hr li main nav noscript ol output p pre section table tfoot ul video}
- def self.apply(string_or_doc, args = nil)
+ def self.apply(string_or_doc, extra_paths: nil)
doc = string_or_doc
doc = Nokogiri::HTML::fragment(doc) if doc.is_a?(String)
changed = false
- each_onebox_link(doc) do |url, element|
+ each_onebox_link(doc, extra_paths: extra_paths) do |url, element|
onebox, _ = yield(url, element)
+
if onebox
parsed_onebox = Nokogiri::HTML::fragment(onebox)
next unless parsed_onebox.children.count > 0
diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb
index fe0fcb45091..83d6714bfe8 100644
--- a/spec/components/cooked_post_processor_spec.rb
+++ b/spec/components/cooked_post_processor_spec.rb
@@ -20,7 +20,6 @@ describe CookedPostProcessor do
it "post process in sequence" do
cpp.expects(:post_process_oneboxes).in_sequence(post_process)
- cpp.expects(:post_process_inline_oneboxes).in_sequence(post_process)
cpp.expects(:post_process_images).in_sequence(post_process)
cpp.expects(:optimize_urls).in_sequence(post_process)
cpp.expects(:pull_hotlinked_images).in_sequence(post_process)
@@ -29,6 +28,87 @@ describe CookedPostProcessor do
expect(PostUpload.exists?(post: post, upload: upload)).to eq(true)
end
+ describe 'when post contains oneboxes and inline oneboxes' do
+ let(:url_hostname) { 'meta.discourse.org' }
+
+ let(:url) do
+ "https://#{url_hostname}/t/mini-inline-onebox-support-rfc/66400"
+ end
+
+ let(:title) { 'some title' }
+
+ let(:post) do
+ Fabricate(:post, raw: <<~RAW)
+ #{url}
+ This is a #{url} with path
+
+ https://#{url_hostname}/t/random-url
+
+ This is a https://#{url_hostname}/t/another-random-url test
+ This is a #{url} with path
+
+ #{url}
+ RAW
+ end
+
+ before do
+ SiteSetting.enable_inline_onebox_on_all_domains = true
+
+ %i{head get}.each do |method|
+ stub_request(method, url).to_return(
+ status: 200,
+ body: <<~RAW
+
+
+ #{title}
+
+
+
+
+ RAW
+ )
+ end
+ end
+
+ after do
+ InlineOneboxer.purge(url)
+ Oneboxer.invalidate(url)
+ end
+
+ it 'should respect SiteSetting.max_oneboxes_per_post' do
+ SiteSetting.max_oneboxes_per_post = 2
+ SiteSetting.add_rel_nofollow_to_user_content = false
+
+ cpp.post_process
+
+ expect(cpp.html).to have_tag('a',
+ with: {
+ href: url,
+ class: described_class::INLINE_ONEBOX_CSS_CLASS
+ },
+ text: title,
+ count: 2
+ )
+
+ expect(cpp.html).to have_tag('aside.onebox a', text: title, count: 2)
+
+ expect(cpp.html).to have_tag('aside.onebox a',
+ text: url_hostname,
+ count: 2
+ )
+
+ expect(cpp.html).to have_tag('a',
+ without: {
+ class: described_class::INLINE_ONEBOX_LOADING_CSS_CLASS
+ },
+ text: "https://#{url_hostname}/t/another-random-url",
+ count: 1
+ )
+
+ expect(cpp.html).to have_tag('a.onebox', count: 1)
+ end
+ end
+
describe 'when post contains inline oneboxes' do
let(:loading_css_class) do
described_class::INLINE_ONEBOX_LOADING_CSS_CLASS
diff --git a/test/javascripts/acceptance/composer-onebox-test.js.es6 b/test/javascripts/acceptance/composer-onebox-test.js.es6
new file mode 100644
index 00000000000..70f529eac9f
--- /dev/null
+++ b/test/javascripts/acceptance/composer-onebox-test.js.es6
@@ -0,0 +1,47 @@
+import { acceptance } from "helpers/qunit-helpers";
+import { INLINE_ONEBOX_CSS_CLASS } from "pretty-text/inline-oneboxer";
+
+acceptance("Composer - Onebox", {
+ loggedIn: true,
+ settings: {
+ max_oneboxes_per_post: 2,
+ enable_markdown_linkify: true
+ }
+});
+
+QUnit.test(
+ "Preview update should respect max_oneboxes_per_post site setting",
+ async assert => {
+ await visit("/t/internationalization-localization/280");
+ await click("#topic-footer-buttons .btn.create");
+
+ await fillIn(
+ ".d-editor-input",
+ `
+http://www.example.com/has-title.html
+This is another test http://www.example.com/has-title.html
+
+http://www.example.com/no-title.html
+
+This is another test http://www.example.com/no-title.html
+This is another test http://www.example.com/has-title.html
+
+http://www.example.com/has-title.html
+ `
+ );
+
+ assert.equal(
+ find(".d-editor-preview:visible")
+ .html()
+ .trim(),
+ `
+
+This is another test This is a great title
+http://www.example.com/no-title.html
+This is another test http://www.example.com/no-title.html
+This is another test This is a great title
+
+ `.trim()
+ );
+ }
+);
diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6
index 09ecf38ed10..20f8acced50 100644
--- a/test/javascripts/helpers/create-pretender.js.es6
+++ b/test/javascripts/helpers/create-pretender.js.es6
@@ -532,6 +532,20 @@ export default function() {
});
});
+ this.get("/inline-onebox", request => {
+ if (
+ request.queryParams.urls.includes(
+ "http://www.example.com/has-title.html"
+ )
+ ) {
+ return [
+ 200,
+ { "Content-Type": "application/html" },
+ '{"inline-oneboxes":[{"url":"http://www.example.com/has-title.html","title":"This is a great title"}]}'
+ ];
+ }
+ });
+
this.get("/onebox", request => {
if (
request.queryParams.url === "http://www.example.com/has-title.html" ||
diff --git a/test/javascripts/helpers/qunit-helpers.js.es6 b/test/javascripts/helpers/qunit-helpers.js.es6
index ca775770a74..7799e087bc4 100644
--- a/test/javascripts/helpers/qunit-helpers.js.es6
+++ b/test/javascripts/helpers/qunit-helpers.js.es6
@@ -13,6 +13,7 @@ import { flushMap } from "discourse/models/store";
import { clearRewrites } from "discourse/lib/url";
import { initSearchData } from "discourse/widgets/search-menu";
import { resetDecorators } from "discourse/widgets/widget";
+import { resetCache as resetOneboxCache } from "pretty-text/oneboxer";
import { resetCustomPostMessageCallbacks } from "discourse/controllers/topic";
export function currentUser() {
@@ -122,6 +123,7 @@ export function acceptance(name, options) {
clearRewrites();
initSearchData();
resetDecorators();
+ resetOneboxCache();
resetCustomPostMessageCallbacks();
Discourse.reset();
}