FIX: Clicking on a URL with a different url prefix did not work (#13349)

FIX: Clicking on a URL with a different url prefix did not work (#13349)

Before this fix if your forum was set up with a subfolder and you clicked on a link to a different subfolder it would not work. For example:

subfolder: /cool link is: /about-us

Previously it would try to resolve /about-us as /cool/about-us. With this fix it redirects to /about-us correctly.

diff --git a/app/assets/javascripts/discourse-common/addon/lib/get-url.js b/app/assets/javascripts/discourse-common/addon/lib/get-url.js
index 9eddfa6..ed6e455 100644
--- a/app/assets/javascripts/discourse-common/addon/lib/get-url.js
+++ b/app/assets/javascripts/discourse-common/addon/lib/get-url.js
@@ -1,6 +1,13 @@
 let cdn, baseUrl, baseUri, baseUriMatcher;
 let S3BaseUrl, S3CDN;
 
+export function getBaseURI() {
+  if (baseUri === undefined) {
+    setPrefix($('meta[name="discourse-base-uri"]').attr("content") || "");
+  }
+  return baseUri || "/";
+}
+
 export default function getURL(url) {
   if (baseUri === undefined) {
     setPrefix($('meta[name="discourse-base-uri"]').attr("content") || "");
diff --git a/app/assets/javascripts/discourse/app/lib/click-track.js b/app/assets/javascripts/discourse/app/lib/click-track.js
index 28744e2..e930f2e 100644
--- a/app/assets/javascripts/discourse/app/lib/click-track.js
+++ b/app/assets/javascripts/discourse/app/lib/click-track.js
@@ -4,7 +4,7 @@ import { Promise } from "rsvp";
 import User from "discourse/models/user";
 import { ajax } from "discourse/lib/ajax";
 import bootbox from "bootbox";
-import getURL from "discourse-common/lib/get-url";
+import getURL, { getBaseURI } from "discourse-common/lib/get-url";
 import { isTesting } from "discourse-common/config/environment";
 import { later } from "@ember/runloop";
 import { selectedText } from "discourse/lib/utilities";
@@ -71,7 +71,7 @@ export function openLinkInNewTab(link) {
 }
 
 export default {
-  trackClick(e, siteSettings) {
+  trackClick(e, siteSettings, { returnPromise = false } = {}) {
     // right clicks are not tracked
     if (e.which === 3) {
       return true;
@@ -162,17 +162,20 @@ export default {
         openLinkInNewTab($link[0]);
       } else {
         trackPromise.finally(() => {
-          if (DiscourseURL.isInternal(href)) {
+          if (
+            DiscourseURL.isInternal(href) &&
+            href.indexOf(getBaseURI()) === 0
+          ) {
             DiscourseURL.routeTo(href);
           } else {
-            DiscourseURL.redirectTo(href);
+            DiscourseURL.redirectAbsolute(href);
           }
         });
       }
 
-      return false;
+      return returnPromise ? trackPromise : false;
     }
 
-    return true;
+    return returnPromise ? trackPromise : true;
   },
 };
diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js
index 67732d5..c412b6b 100644
--- a/app/assets/javascripts/discourse/app/lib/url.js
+++ b/app/assets/javascripts/discourse/app/lib/url.js
@@ -9,6 +9,7 @@ import { defaultHomepage } from "discourse/lib/utilities";
 import { isEmpty } from "@ember/utils";
 import offsetCalculator from "discourse/lib/offset-calculator";
 import { setOwner } from "@ember/application";
+import { isTesting } from "discourse-common/config/environment";
 
 const rewrites = [];
 export const TOPIC_URL_REGEXP = /\/t\/([^\/]+)\/(\d+)\/?(\d+)?/;
@@ -226,7 +227,7 @@ const DiscourseURL = EmberObject.extend({
     }
 
     if (Session.currentProp("requiresRefresh")) {
-      return this.redirectTo(getURL(path));
+      return this.redirectTo(path);
     }
 
     const pathname = path.replace(/(https?\:)?\/\/[^\/]+/, "");
@@ -308,17 +309,20 @@ const DiscourseURL = EmberObject.extend({
     rewrites.push({ regexp, replacement, opts: opts || {} });
   },
 
-  redirectTo(url) {
-    window.location = getURL(url);
+  redirectAbsolute(url) {
+    // Redirects will kill a test runner
+    if (isTesting()) {
+      return true;
+    }
+    window.location = url;
     return true;
   },
 
-  /**
-   * Determines whether a URL is internal or not
-   *
-   * @method isInternal
-   * @param {String} url
-   **/
+  redirectTo(url) {
+    return this.redirectAbsolute(getURL(url));
+  },
+
+  // Determines whether a URL is internal or not
   isInternal(url) {
     if (url && url.length) {
       if (url.indexOf("//") === 0) {
diff --git a/app/assets/javascripts/discourse/tests/index.html b/app/assets/javascripts/discourse/tests/index.html
index 2a70fa6..4660e3e 100644
--- a/app/assets/javascripts/discourse/tests/index.html
+++ b/app/assets/javascripts/discourse/tests/index.html
@@ -26,6 +26,14 @@
         -o-transition: none !important;
         transition: none !important;
       }
+      #qunit-fixture {
+        position: absolute;
+        top: -10000px;
+        left: -10000px;
+        width: 1000px;
+        height: 1000px;
+      }
+
     </style>
 
     <script src="{{rootURL}}assets/test-i18n.js"></script>
diff --git a/app/assets/javascripts/discourse/tests/unit/lib/click-track-test.js b/app/assets/javascripts/discourse/tests/unit/lib/click-track-test.js
index 14b0b66..3327e94 100644
--- a/app/assets/javascripts/discourse/tests/unit/lib/click-track-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/lib/click-track-test.js
@@ -6,6 +6,7 @@ import User from "discourse/models/user";
 import { later } from "@ember/runloop";
 import pretender from "discourse/tests/helpers/create-pretender";
 import sinon from "sinon";
+import { setPrefix } from "discourse-common/lib/get-url";
 
 const track = ClickTrack.trackClick;
 
@@ -47,6 +48,8 @@ module("Unit | Utility | click-track", function (hooks) {
             <a href="https://discuss.domain.com/t/welcome-to-meta-discourse-org/1/30">foo</a>
             <a href="https://google.com">bar</a>
           </aside>
+          <a class="prefix-url" href="/forum/thing">prefix link</a>
+          <a class="diff-prefix-url" href="/thing">diff prefix link</a>
         </article>
       </div>`
     );
@@ -85,6 +88,26 @@ module("Unit | Utility | click-track", function (hooks) {
     );
   });
 
+  test("routes to internal urls", async function (assert) {
+    setPrefix("/forum");
+    pretender.post("/clicks/track", () => [200, {}, ""]);
+    await track(generateClickEventOn(".prefix-url"), null, {
+      returnPromise: true,
+    });
+    assert.ok(DiscourseURL.routeTo.calledWith("/forum/thing"));
+  });
+
+  test("redirects to internal urls with a different prefix", async function (assert) {
+    setPrefix("/forum");
+    sinon.stub(DiscourseURL, "redirectAbsolute");
+
+    pretender.post("/clicks/track", () => [200, {}, ""]);
+    await track(generateClickEventOn(".diff-prefix-url"), null, {
+      returnPromise: true,
+    });
+    assert.ok(DiscourseURL.redirectAbsolute.calledWith("/thing"));
+  });
+
   skip("tracks external URLs", async function (assert) {
     assert.expect(2);
 

GitHub sha: 052c841550ad5dd42fdf979e0c4ad57eb960e3be

This commit appears in #13349 which was approved by SamSaffron. It was merged by SamSaffron.