FEATURE: onebox everything by default

FEATURE: new 'max_oneboxes_per_post' site setting
FEATURE: change onebox whitelist to a blacklist
PERF: debounce the loading of oneboxes
PERF: improve perf of mention links in preview
FIX: sort loading of custom oneboxer

app/assets/javascripts/discourse/components/composer-editor.js.es6

@@ -2,7 +2,7 @@ import userSearch from 'discourse/lib/user-search';
 import { default as computed, on } from 'ember-addons/ember-computed-decorators';
 import { linkSeenMentions, fetchUnseenMentions } from 'discourse/lib/link-mentions';
 import { linkSeenCategoryHashtags, fetchUnseenCategoryHashtags } from 'discourse/lib/link-category-hashtags';
-import { fetchUnseenTagHashtags, linkSeenTagHashtags } from 'discourse/lib/link-tag-hashtag';
+import { linkSeenTagHashtags, fetchUnseenTagHashtags } from 'discourse/lib/link-tag-hashtag';
 import { load } from 'pretty-text/oneboxer';
 import { ajax } from 'discourse/lib/ajax';
 import InputValidation from 'discourse/models/input-validation';
@@ -41,22 +41,6 @@ export default Ember.Component.extend({
     return showPreview ? I18n.t('composer.hide_preview') : I18n.t('composer.show_preview');
   },
 
-  _renderUnseenTagHashtags($preview, unseen) {
-    fetchUnseenTagHashtags(unseen).then(() => {
-      linkSeenTagHashtags($preview);
-    });
-  },
-
-  @on('previewRefreshed')
-  paintTagHashtags($preview) {
-    if (!this.siteSettings.tagging_enabled) { return; }
-
-    const unseenTagHashtags = linkSeenTagHashtags($preview);
-    if (unseenTagHashtags.length) {
-      Ember.run.debounce(this, this._renderUnseenTagHashtags, $preview, unseenTagHashtags, 500);
-    }
-  },
-
   @computed
   markdownOptions() {
     return {
@@ -152,19 +136,38 @@ export default Ember.Component.extend({
     $preview.scrollTop(desired + 50);
   },
 
-  _renderUnseenMentions: function($preview, unseen) {
-    fetchUnseenMentions($preview, unseen).then(() => {
+  _renderUnseenMentions($preview, unseen) {
+    fetchUnseenMentions(unseen).then(() => {
       linkSeenMentions($preview, this.siteSettings);
       this._warnMentionedGroups($preview);
     });
   },
 
-  _renderUnseenCategoryHashtags: function($preview, unseen) {
+  _renderUnseenCategoryHashtags($preview, unseen) {
     fetchUnseenCategoryHashtags(unseen).then(() => {
       linkSeenCategoryHashtags($preview);
     });
   },
 
+  _renderUnseenTagHashtags($preview, unseen) {
+    fetchUnseenTagHashtags(unseen).then(() => {
+      linkSeenTagHashtags($preview);
+    });
+  },
+
+  _loadOneboxes($oneboxes) {
+    const post = this.get('composer.post');
+    let refresh = false;
+
+    // If we are editing a post, we'll refresh its contents once.
+    if (post && !post.get('refreshedPost')) {
+      refresh = true;
+      post.set('refreshedPost', true);
+    }
+
+    $oneboxes.each((_, o) => load(o, refresh, ajax));
+  },
+
   _warnMentionedGroups($preview) {
     Ember.run.scheduleOnce('afterRender', () => {
       this._warnedMentions = this._warnedMentions || [];
@@ -481,31 +484,33 @@ export default Ember.Component.extend({
 
     previewUpdated($preview) {
       // Paint mentions
-      const unseen = linkSeenMentions($preview, this.siteSettings);
-      if (unseen.length) {
-        Ember.run.debounce(this, this._renderUnseenMentions, $preview, unseen, 500);
+      const unseenMentions = linkSeenMentions($preview, this.siteSettings);
+      if (unseenMentions.length) {
+        Ember.run.debounce(this, this._renderUnseenMentions, $preview, unseenMentions, 450);
       }
 
       this._warnMentionedGroups($preview);
 
       // Paint category hashtags
-      const unseenHashtags = linkSeenCategoryHashtags($preview);
-      if (unseenHashtags.length) {
-        Ember.run.debounce(this, this._renderUnseenCategoryHashtags, $preview, unseenHashtags, 500);
+      const unseenCategoryHashtags = linkSeenCategoryHashtags($preview);
+      if (unseenCategoryHashtags.length) {
+        Ember.run.debounce(this, this._renderUnseenCategoryHashtags, $preview, unseenCategoryHashtags, 450);
       }
 
-      const post = this.get('composer.post');
-      let refresh = false;
-
-      // If we are editing a post, we'll refresh its contents once. This is a feature that
-      // allows a user to refresh its contents once.
-      if (post && !post.get('refreshedPost')) {
-        refresh = true;
-        post.set('refreshedPost', true);
+      // Paint tag hashtags
+      if (this.siteSettings.tagging_enabled) {
+        const unseenTagHashtags = linkSeenTagHashtags($preview);
+        if (unseenTagHashtags.length) {
+          Ember.run.debounce(this, this._renderUnseenTagHashtags, $preview, unseenTagHashtags, 450);
+        }
       }
 
       // Paint oneboxes
-      $('a.onebox', $preview).each((i, e) => load(e, refresh, ajax));
+      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);
+      }
+
       this.trigger('previewRefreshed', $preview);
       this.sendAction('afterRefresh', $preview);
     },

app/assets/javascripts/discourse/lib/link-mentions.js.es6

@@ -1,37 +1,34 @@
 import { ajax } from 'discourse/lib/ajax';
+
 function replaceSpan($e, username, opts) {
   if (opts && opts.group) {
-    var extra = "", extraClass = "";
+    let extra = "";
+    let extraClass = "";
     if (opts.mentionable) {
-      extra = " data-name='" + username + "' data-mentionable-user-count='" + opts.mentionable.user_count + "' ";
-      extraClass = " notify";
+      extra = `data-name='${username}' data-mentionable-user-count='${opts.mentionable.user_count}'`;
+      extraClass = "notify";
     }
-    $e.replaceWith("<a href='" +
-                  Discourse.getURL("/groups/") + username +
-                  "' class='mention-group" + extraClass + "'" + extra + ">@" + username + "</a>");
+    $e.replaceWith(`<a href='${Discourse.getURL("/groups/") + username}' class='mention-group ${extraClass}' ${extra}>@${username}</a>`);
   } else {
-    $e.replaceWith("<a href='" +
-                  Discourse.getURL("/users/") + username.toLowerCase() +
-                  "' class='mention'>@" + username + "</a>");
+    $e.replaceWith(`<a href='${Discourse.getURL("/users/") + username.toLowerCase()}' class='mention'>@${username}</a>`);
   }
 }
 
-const found = [];
-const foundGroups = [];
-const mentionableGroups = [];
-const checked = [];
+const found = {};
+const foundGroups = {};
+const mentionableGroups = {};
+const checked = {};
 
 function updateFound($mentions, usernames) {
   Ember.run.scheduleOnce('afterRender', function() {
     $mentions.each((i, e) => {
       const $e = $(e);
       const username = usernames[i];
-      if (found.indexOf(username.toLowerCase()) !== -1) {
+      if (found[username.toLowerCase()]) {
         replaceSpan($e, username);
-      } else if (foundGroups.indexOf(username) !== -1) {
-        const mentionable = _(mentionableGroups).where({name: username}).first();
-        replaceSpan($e, username, {group: true, mentionable: mentionable});
-      } else if (checked.indexOf(username) !== -1) {
+      } else if (foundGroups[username]) {
+        replaceSpan($e, username, { group: true, mentionable: mentionableGroups[username] });
+      } else if (checked[username]) {
         $e.addClass('mention-tested');
       }
     });
@@ -42,22 +39,18 @@ export function linkSeenMentions($elem, siteSettings) {
   const $mentions = $('span.mention:not(.mention-tested)', $elem);
   if ($mentions.length) {
     const usernames = $mentions.map((_, e) => $(e).text().substr(1));
-    const unseen = _.uniq(usernames).filter((u) => {
-      return u.length >= siteSettings.min_username_length && checked.indexOf(u) === -1;
-    });
     updateFound($mentions, usernames);
-    return unseen;
+    return _.uniq(usernames).filter(u => !checked[u] && u.length >= siteSettings.min_username_length);
   }
-
   return [];
 }
 
-export function fetchUnseenMentions($elem, usernames) {
-  return ajax("/users/is_local_username", { data: { usernames } }).then(function(r) {
-    found.push.apply(found, r.valid);
-    foundGroups.push.apply(foundGroups, r.valid_groups);
-    mentionableGroups.push.apply(mentionableGroups, r.mentionable_groups);
-    checked.push.apply(checked, usernames);
+export function fetchUnseenMentions(usernames) {
+  return ajax("/users/is_local_username", { data: { usernames } }).then(r => {
+    r.valid.forEach(v => found[v] = true);
+    r.valid_groups.forEach(vg => foundGroups[vg] = true);
+    r.mentionable_groups.forEach(mg => mentionableGroups[mg] = true);
+    usernames.forEach(u => checked[u] = true);
     return r;
   });
 }

app/assets/javascripts/pretty-text/oneboxer.js.es6

@@ -1,19 +1,12 @@
-/**
-  A helper for looking up oneboxes and displaying them
-
-  For now it only stores in a local Javascript Object, in future we can change it so it uses localStorage
-  or some other mechanism.
-**/
-
 const localCache = {};
 const failedCache = {};
 
-// Perform a lookup of a onebox based an anchor element. It will insert a loading
-// indicator and remove it when the loading is complete or fails.
+// Perform a lookup of a onebox based an anchor element.
+// It will insert a loading indicator and remove it when the loading is complete or fails.
 export function load(e, refresh, ajax) {
-  var $elem = $(e);
+  const $elem = $(e);
 
-  // If the onebox has loaded, return
+  // If the onebox has loaded or is loading, return
   if ($elem.data('onebox-loaded')) return;
   if ($elem.hasClass('loading-onebox')) return;
 
@@ -41,7 +34,7 @@ export function load(e, refresh, ajax) {
   }).then(html => {
     localCache[url] = html;
     $elem.replaceWith(html);
-  }, function() {
+  }, () => {
     failedCache[url] = true;
   }).finally(() => {
     $elem.removeClass('loading-onebox');

config/locales/server.en.yml

@@ -816,7 +816,7 @@ en:
     s3_config_warning: 'The server is configured to upload files to s3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key or s3_upload_bucket. Go to <a href="/admin/site_settings">the Site Settings</a> and update the settings. <a href="http://meta.discourse.org/t/how-to-set-up-image-uploads-to-s3/7229" target="_blank">See "How to set up image uploads to S3?" to learn more</a>.'
     s3_backup_config_warning: 'The server is configured to upload backups to s3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key or s3_backup_bucket. Go to <a href="/admin/site_settings">the Site Settings</a> and update the settings. <a href="http://meta.discourse.org/t/how-to-set-up-image-uploads-to-s3/7229" target="_blank">See "How to set up image uploads to S3?" to learn more</a>.'
     image_magick_warning: 'The server is configured to create thumbnails of large images, but ImageMagick is not installed. Install ImageMagick using your favorite package manager or <a href="http://www.imagemagick.org/script/binary-releases.php" target="_blank">download the latest release</a>.'
-    failing_emails_warning: 'There are %{num_failed_jobs} email jobs that failed. Check your app.yml and ensure that the mail server settings are correct. <a href="/sidekiq/retries" target="_blank">See the failed jobs in Sidekiq</a>.'  
+    failing_emails_warning: 'There are %{num_failed_jobs} email jobs that failed. Check your app.yml and ensure that the mail server settings are correct. <a href="/sidekiq/retries" target="_blank">See the failed jobs in Sidekiq</a>.'
     subfolder_ends_in_slash: "Your subfolder setup is incorrect; the DISCOURSE_RELATIVE_URL_ROOT ends in a slash."
     email_polling_errored_recently:
       one: "Email polling has generated an error in the past 24 hours. Look at <a href='/logs' target='_blank'>the logs</a> for more details."
@@ -874,7 +874,8 @@ en:
     show_pinned_excerpt_mobile: "Show excerpt on pinned topics in mobile view."
     show_pinned_excerpt_desktop: "Show excerpt on pinned topics in desktop view."
     post_onebox_maxlength: "Maximum length of a oneboxed Discourse post in characters."
-    onebox_domains_whitelist: "A list of domains to allow oneboxing for; these domains should support OpenGraph or oEmbed. Test them at http://iframely.com/debug"
+    onebox_domains_blacklist: "A list of domains that will never be oneboxed."
+    max_oneboxes_per_post: "Maximum number of oneboxes in a post."
 
     logo_url: "The logo image at the top left of your site, should be a wide rectangle shape. If left blank site title text will be shown."
     digest_logo_url: "The alternate logo image used at the top of your site's email summary. Should be a wide rectangle shape. Should not be an SVG image. If left blank `logo_url` will be used."

config/site_settings.yml

@@ -855,9 +855,12 @@ security:
 onebox:
   enable_flash_video_onebox: false
   post_onebox_maxlength: 500
-  onebox_domains_whitelist:
+  onebox_domains_blacklist:
     default: ''
     type: list
+  max_oneboxes_per_post:
+    default: 50
+    client: true
 
 spam:
   add_rel_nofollow_to_user_content: true

lib/oneboxer.rb

@@ -1,11 +1,8 @@
-
-Dir["#{Rails.root}/lib/onebox/engine/*_onebox.rb"].each {|f|
+Dir["#{Rails.root}/lib/onebox/engine/*_onebox.rb"].sort.each {|f|
   require_dependency(f.split('/')[-3..-1].join('/'))
 }
 
 module Oneboxer
-
-
   # keep reloaders happy
   unless defined? Oneboxer::Result
     Result = Struct.new(:doc, :changed) do
@@ -120,38 +117,29 @@ module Oneboxer
   end
 
   private
-  def self.onebox_cache_key(url)
-    "onebox__#{url}"
-  end
 
-  def self.add_discourse_whitelists
-    # Add custom domain whitelists
-    if SiteSetting.onebox_domains_whitelist.present?
-      domains = SiteSetting.onebox_domains_whitelist.split('|')
-      whitelist = Onebox::Engine::WhitelistedGenericOnebox.whitelist
-      whitelist.concat(domains)
-      whitelist.uniq!
+    def self.blank_onebox
+      { preview: "", onebox: "" }
     end
-  end
 
-  def self.onebox_raw(url)
-    Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day){
-      # This might be able to move to whenever the SiteSetting changes?
-      Oneboxer.add_discourse_whitelists
+    def self.onebox_cache_key(url)
+      "onebox__#{url}"
+    end
 
-      r = Onebox.preview(url, cache: {}, max_width: 695)
-      {
-        onebox: r.to_s,
-        preview: r.try(:placeholder_html).to_s
-      }
-    }
-  rescue => e
-    # no point warning here, just cause we have an issue oneboxing a url
-    # we can later hunt for failed oneboxes by searching logs if needed
-    Rails.logger.info("Failed to onebox #{url} #{e} #{e.backtrace}")
+    def self.onebox_raw(url)
+      Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day) do
+        uri = URI(url) rescue nil
+        return blank_onebox if uri.blank? || SiteSetting.onebox_domains_blacklist.include?(uri.hostname)
 
-    # return a blank hash, so rest of the code works
-    {preview: "", onebox: ""}
-  end
+        r = Onebox.preview(url, cache: {}, max_width: 695)
+        { onebox: r.to_s, preview: r.try(:placeholder_html).to_s }
+      end
+    rescue => e
+      # no point warning here, just cause we have an issue oneboxing a url
+      # we can later hunt for failed oneboxes by searching logs if needed
+      Rails.logger.info("Failed to onebox #{url} #{e} #{e.backtrace}")
+      # return a blank hash, so rest of the code works
+      blank_onebox
+    end
 
 end

lib/site_setting_extension.rb

@@ -348,12 +348,9 @@ module SiteSettingExtension
   end
 
   def filter_value(name, value)
-    # filter domain name
-    if %w[disabled_image_download_domains onebox_domains_whitelist exclude_rel_nofollow_domains email_domains_blacklist email_domains_whitelist white_listed_spam_host_domains].include? name
+    if %w[disabled_image_download_domains onebox_domains_blacklist exclude_rel_nofollow_domains email_domains_blacklist email_domains_whitelist white_listed_spam_host_domains].include? name
       domain_array = []
-      value.split('|').each { |url|
-        domain_array.push(get_hostname(url))
-      }
+      value.split('|').each { |url| domain_array << get_hostname(url) }
       value = domain_array.join("|")
     end
     value

spec/components/oneboxer_spec.rb

@@ -2,9 +2,11 @@ require 'rails_helper'
 require_dependency 'oneboxer'
 
 describe Oneboxer do
+
   it "returns blank string for an invalid onebox" do
     expect(Oneboxer.preview("http://boom.com")).to eq("")
     expect(Oneboxer.onebox("http://boom.com")).to eq("")
   end
+
 end