From 8e28ccb2ea88c107be48aa1e55d311ca7a9fb458 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Thu, 23 Apr 2020 11:03:46 +0100 Subject: [PATCH] PERF: Refactor decorateCooked to run in a detached DOM (#9517) This means that decorateCooked can be used to modify HTML without triggering the download of remote resources (e.g. images) In some rare cases (e.g. IntersectionObservers in Chromium), decorating needs to happen in the real DOM. For this, pass `afterAdopt: true` to `decorateCooked` --- .../discourse/lib/lazy-load-images.js | 31 +++++--- .../javascripts/discourse/lib/plugin-api.js | 7 +- .../discourse/widgets/post-cooked.js | 71 +++++++++++++------ 3 files changed, 78 insertions(+), 31 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/lazy-load-images.js b/app/assets/javascripts/discourse/lib/lazy-load-images.js index f326b2ebf25..56ab825e0b8 100644 --- a/app/assets/javascripts/discourse/lib/lazy-load-images.js +++ b/app/assets/javascripts/discourse/lib/lazy-load-images.js @@ -84,6 +84,14 @@ function show(image) { } } +function forEachImage($post, callback) { + $post[0].querySelectorAll("img").forEach(img => { + if (img.width >= MINIMUM_SIZE && img.height >= MINIMUM_SIZE) { + callback(img); + } + }); +} + export function setupLazyLoading(api) { const observer = new IntersectionObserver(entries => { entries.forEach(entry => { @@ -96,15 +104,20 @@ export function setupLazyLoading(api) { }); }, OBSERVER_OPTIONS); + api.decorateCooked($post => forEachImage($post, img => hide(img)), { + onlyStream: true, + id: "discourse-lazy-load" + }); + + // IntersectionObserver.observe must be called after the cooked + // content is adopted by the document element in chrome + // https://bugs.chromium.org/p/chromium/issues/detail?id=1073469 api.decorateCooked( - $post => { - $("img", $post).each((_, img) => { - if (img.width >= MINIMUM_SIZE && img.height >= MINIMUM_SIZE) { - hide(img); - observer.observe(img); - } - }); - }, - { onlyStream: true, id: "discourse-lazy-load" } + $post => forEachImage($post, img => observer.observe(img)), + { + onlyStream: true, + id: "discourse-lazy-load-after-adopt", + afterAdopt: true + } ); } diff --git a/app/assets/javascripts/discourse/lib/plugin-api.js b/app/assets/javascripts/discourse/lib/plugin-api.js index 6df49e453d2..8f0e14253fa 100644 --- a/app/assets/javascripts/discourse/lib/plugin-api.js +++ b/app/assets/javascripts/discourse/lib/plugin-api.js @@ -54,7 +54,7 @@ import { on } from "@ember/object/evented"; import KeyboardShortcuts from "discourse/lib/keyboard-shortcuts"; // If you add any methods to the API ensure you bump up this number -const PLUGIN_API_VERSION = "0.8.40"; +const PLUGIN_API_VERSION = "0.8.41"; class PluginApi { constructor(version, container) { @@ -200,6 +200,9 @@ class PluginApi { * Use `options.onlyStream` if you only want to decorate posts within a topic, * and not in other places like the user stream. * + * Decoration normally happens in a detached DOM. Use `options.afterAdopt` + * to decorate html content after it is adopted by the main `document`. + * * For example, to add a yellow background to all posts you could do this: * * ``` @@ -215,7 +218,7 @@ class PluginApi { decorateCooked(callback, opts) { opts = opts || {}; - addDecorator(callback); + addDecorator(callback, { afterAdopt: !!opts.afterAdopt }); if (!opts.onlyStream) { decorate(ComposerEditor, "previewRefreshed", callback, opts.id); diff --git a/app/assets/javascripts/discourse/widgets/post-cooked.js b/app/assets/javascripts/discourse/widgets/post-cooked.js index f587a76817d..71fc9f43ef0 100644 --- a/app/assets/javascripts/discourse/widgets/post-cooked.js +++ b/app/assets/javascripts/discourse/widgets/post-cooked.js @@ -8,15 +8,27 @@ import { unhighlightHTML } from "discourse/lib/highlight-html"; -let _decorators = []; +let _beforeAdoptDecorators = []; +let _afterAdoptDecorators = []; // Don't call this directly: use `plugin-api/decorateCooked` -export function addDecorator(cb) { - _decorators.push(cb); +export function addDecorator(callback, { afterAdopt = false } = {}) { + if (afterAdopt) { + _afterAdoptDecorators.push(callback); + } else { + _beforeAdoptDecorators.push(callback); + } } export function resetDecorators() { - _decorators = []; + _beforeAdoptDecorators = []; + _afterAdoptDecorators = []; +} + +let detachedDocument = document.implementation.createHTMLDocument("detached"); + +function createDetachedElement(nodeName) { + return detachedDocument.createElement(nodeName); } export default class PostCooked { @@ -41,14 +53,27 @@ export default class PostCooked { } init() { - const $html = this._computeCooked(); - this._insertQuoteControls($html); - this._showLinkCounts($html); - this._fixImageSizes($html); - this._applySearchHighlight($html); + const cookedDiv = this._computeCooked(); + const $cookedDiv = $(cookedDiv); - _decorators.forEach(cb => cb($html, this.decoratorHelper)); - return $html[0]; + this._insertQuoteControls($cookedDiv); + this._showLinkCounts($cookedDiv); + this._fixImageSizes($cookedDiv); + this._applySearchHighlight($cookedDiv); + + this._decorateAndAdopt(cookedDiv); + + return cookedDiv; + } + + _decorateAndAdopt(cooked) { + const $cooked = $(cooked); + + _beforeAdoptDecorators.forEach(d => d($cooked, this.decoratorHelper)); + + document.adoptNode(cooked); + + _afterAdoptDecorators.forEach(d => d($cooked, this.decoratorHelper)); } _applySearchHighlight($html) { @@ -175,12 +200,14 @@ export default class PostCooked { quotedPosts[result.id] = result; post.set("quoted", quotedPosts); - const div = $("
"); - div.data("post-id", result.id); - div.html(result.cooked); - _decorators.forEach(cb => cb(div, this.decoratorHelper)); + const div = createDetachedElement("div"); + div.classList.add("expanded-quote"); + div.dataset.postId = result.id; + div.innerHTML = result.cooked; - highlightHTML(div[0], originalText, { + this._decorateAndAdopt(div); + + highlightHTML(div, originalText, { matchCase: true }); $blockQuote.showHtml(div, "fast", finished); @@ -275,18 +302,22 @@ export default class PostCooked { } _computeCooked() { + const cookedDiv = createDetachedElement("div"); + cookedDiv.classList.add("cooked"); + if ( (this.attrs.firstPost || this.attrs.embeddedPost) && this.ignoredUsers && this.ignoredUsers.length > 0 && this.ignoredUsers.includes(this.attrs.username) ) { - return $( - `
${I18n.t("post.ignored")}
` - ); + cookedDiv.classList.add("post-ignored"); + cookedDiv.innerHTML = I18n.t("post.ignored"); + } else { + cookedDiv.innerHTML = this.attrs.cooked; } - return $(`
${this.attrs.cooked}
`); + return cookedDiv; } }