REFACTOR: Remove jquery usage from resolveAllShortUrls, and fix debounce

- This function now requires an explicit scope. It will never run on the entire document.

- Previously debounce was being used with an anonymous function, which means it was having no effect.

app/assets/javascripts/discourse/app/components/cook-text.js

@@ -16,7 +16,7 @@ const CookText = Component.extend({
       next(() =>
         window
           .requireModule("pretty-text/upload-short-url")
-          .resolveAllShortUrls(ajax, this.siteSettings)
+          .resolveAllShortUrls(ajax, this.siteSettings, this.element)
       );
     });
   }

app/assets/javascripts/pretty-text/addon/upload-short-url.js

@@ -43,12 +43,14 @@ export function resetCache() {
   _cache = {};
 }
 
-function retrieveCachedUrl($upload, siteSettings, dataAttribute, callback) {
-  const cachedUpload = lookupCachedUploadUrl($upload.data(dataAttribute));
+function retrieveCachedUrl(upload, siteSettings, dataAttribute, callback) {
+  const cachedUpload = lookupCachedUploadUrl(
+    upload.getAttribute(`data-${dataAttribute}`)
+  );
   const url = getAttributeBasedUrl(dataAttribute, cachedUpload, siteSettings);
 
   if (url) {
-    $upload.removeAttr(`data-${dataAttribute}`);
+    upload.removeAttribute(`data-${dataAttribute}`);
     if (url !== MISSING) {
       callback(url);
     }
@@ -77,43 +79,40 @@ function getAttributeBasedUrl(dataAttribute, cachedUpload, siteSettings) {
   return cachedUpload.short_path;
 }
 
-function _loadCachedShortUrls($uploads, siteSettings) {
-  $uploads.each((_idx, upload) => {
-    const $upload = $(upload);
+function _loadCachedShortUrls(uploadElements, siteSettings) {
+  uploadElements.forEach(upload => {
     switch (upload.tagName) {
       case "A":
-        retrieveCachedUrl($upload, siteSettings, "orig-href", url => {
-          $upload.attr("href", url);
+        retrieveCachedUrl(upload, siteSettings, "orig-href", url => {
+          upload.href = url;
         });
 
         break;
       case "IMG":
-        retrieveCachedUrl($upload, siteSettings, "orig-src", url => {
-          $upload.attr("src", url);
+        retrieveCachedUrl(upload, siteSettings, "orig-src", url => {
+          upload.src = url;
         });
 
         break;
       case "SOURCE": // video/audio tag > source tag
-        retrieveCachedUrl($upload, siteSettings, "orig-src", url => {
-          $upload.attr("src", url);
-
+        retrieveCachedUrl(upload, siteSettings, "orig-src", url => {
           if (url.startsWith(`//${window.location.host}`)) {
             let hostRegex = new RegExp("//" + window.location.host, "g");
             url = url.replace(hostRegex, "");
           }
           let fullUrl = window.location.origin + url;
-          $upload.attr("src", fullUrl);
+          upload.src = fullUrl;
 
           // this is necessary, otherwise because of the src change the
           // video/audio just doesn't bother loading!
-          let $parent = $upload.parent();
-          $parent[0].load();
+          upload.parentElement.load();
 
           // set the url and text for the <a> tag within the <video/audio> tag
-          $parent
-            .find("a")
-            .attr("href", fullUrl)
-            .text(fullUrl);
+          const link = upload.parentElement.querySelector("a");
+          if (link) {
+            link.href = fullUrl;
+            link.textContent = fullUrl;
+          }
         });
 
         break;
@@ -121,31 +120,36 @@ function _loadCachedShortUrls($uploads, siteSettings) {
   });
 }
 
-function _loadShortUrls($uploads, ajax, siteSettings) {
-  let urls = $uploads.toArray().map(upload => {
-    const $upload = $(upload);
-    return $upload.data("orig-src") || $upload.data("orig-href");
+function _loadShortUrls(uploads, ajax, siteSettings) {
+  let urls = [...uploads].map(upload => {
+    return (
+      upload.getAttribute("data-orig-src") ||
+      upload.getAttribute("data-orig-href")
+    );
   });
 
   return lookupUncachedUploadUrls(urls, ajax).then(() =>
-    _loadCachedShortUrls($uploads, siteSettings)
+    _loadCachedShortUrls(uploads, siteSettings)
   );
 }
 
-export function resolveAllShortUrls(ajax, siteSettings, scope = null) {
+export function resolveAllShortUrls(ajax, siteSettings, scope) {
   const attributes =
     "img[data-orig-src], a[data-orig-href], source[data-orig-src]";
-  let $shortUploadUrls = $(scope || document).find(attributes);
+  let shortUploadElements = scope.querySelectorAll(attributes);
 
-  if ($shortUploadUrls.length > 0) {
-    _loadCachedShortUrls($shortUploadUrls, siteSettings);
+  if (shortUploadElements.length > 0) {
+    _loadCachedShortUrls(shortUploadElements, siteSettings);
 
-    $shortUploadUrls = $(scope || document).find(attributes);
-    if ($shortUploadUrls.length > 0) {
+    shortUploadElements = scope.querySelectorAll(attributes);
+    if (shortUploadElements.length > 0) {
       // this is carefully batched so we can do a leading debounce (trigger right away)
       return debounce(
         null,
-        () => _loadShortUrls($shortUploadUrls, ajax, siteSettings),
+        _loadShortUrls,
+        shortUploadElements,
+        ajax,
+        siteSettings,
         450,
         true
       );

test/javascripts/lib/upload-short-url-test.js

@@ -84,7 +84,7 @@ QUnit.test("resolveAllShortUrls", async assert => {
   lookup = lookupCachedUploadUrl("upload://a.jpeg");
   assert.deepEqual(lookup, {});
 
-  await resolveAllShortUrls(ajax, { secure_media: false });
+  await resolveAllShortUrls(ajax, { secure_media: false }, fixture()[0]);
 
   lookup = lookupCachedUploadUrl("upload://a.jpeg");
 
@@ -126,7 +126,7 @@ QUnit.test(
   "resolveAllShortUrls - href + src replaced correctly",
   async assert => {
     stubUrls();
-    await resolveAllShortUrls(ajax, { secure_media: false });
+    await resolveAllShortUrls(ajax, { secure_media: false }, fixture()[0]);
 
     let image1 = fixture()
       .find("img")
@@ -156,7 +156,7 @@ QUnit.test(
       ],
       null
     );
-    await resolveAllShortUrls(ajax, { secure_media: true });
+    await resolveAllShortUrls(ajax, { secure_media: true }, fixture()[0]);
 
     let link = fixture().find("a");
     assert.equal(
@@ -169,7 +169,9 @@ QUnit.test(
 QUnit.test("resolveAllShortUrls - scoped", async assert => {
   stubUrls();
   let lookup;
-  await resolveAllShortUrls(ajax, ".scoped-area");
+
+  let scopedElement = fixture()[0].querySelector(".scoped-area");
+  await resolveAllShortUrls(ajax, {}, scopedElement);
 
   lookup = lookupCachedUploadUrl("upload://z.jpeg");
 
@@ -181,7 +183,7 @@ QUnit.test("resolveAllShortUrls - scoped", async assert => {
   // do this because the pretender caches ALL the urls, not
   // just the ones being looked up (like the normal behaviour)
   resetCache();
-  await resolveAllShortUrls(ajax, ".scoped-area");
+  await resolveAllShortUrls(ajax, {}, scopedElement);
 
   lookup = lookupCachedUploadUrl("upload://a.jpeg");
   assert.deepEqual(lookup, {});