From 2704a02e3a23401f8225a3bd1ceebc528ee43919 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Thu, 15 Sep 2022 08:09:34 -0400
Subject: [PATCH] FIX: In-page anchor links were broken in subfolder setups
 (#18250)

The key fix in this commit is that it removes `this.replaceState(path)` for anchor-only URLs. We still intercept those routing changes to properly calculate the scroll position of the anchor via `jumpToElement`, but we no longer use the Ember router to override the browser's history. This fixes the subfolder issue and also lets the browser maintain its history correctly.

The commit also includes a small refactor to the `jumpToElement` helper to facilitate stubbing in tests.
---
 .../discourse/app/lib/intercept-click.js      |  2 +-
 .../discourse/app/lib/static-route-builder.js |  4 +-
 .../javascripts/discourse/app/lib/url.js      | 53 ++++++++++---------
 .../discourse/tests/unit/lib/url-test.js      |  9 ++++
 4 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/lib/intercept-click.js b/app/assets/javascripts/discourse/app/lib/intercept-click.js
index 62d91484ed9..0e7b33334af 100644
--- a/app/assets/javascripts/discourse/app/lib/intercept-click.js
+++ b/app/assets/javascripts/discourse/app/lib/intercept-click.js
@@ -27,7 +27,7 @@ export default function interceptClick(e) {
 
   if (
     !href ||
-    href === "#" ||
+    href.startsWith("#") ||
     currentTarget.getAttribute("target") ||
     currentTarget.dataset.emberAction ||
     currentTarget.dataset.autoRoute ||
diff --git a/app/assets/javascripts/discourse/app/lib/static-route-builder.js b/app/assets/javascripts/discourse/app/lib/static-route-builder.js
index bf5862a5db9..274e1e5186b 100644
--- a/app/assets/javascripts/discourse/app/lib/static-route-builder.js
+++ b/app/assets/javascripts/discourse/app/lib/static-route-builder.js
@@ -1,4 +1,4 @@
-import DiscourseURL, { jumpToElement } from "discourse/lib/url";
+import DiscourseURL from "discourse/lib/url";
 import DiscourseRoute from "discourse/routes/discourse";
 import I18n from "I18n";
 import StaticPage from "discourse/models/static-page";
@@ -25,7 +25,7 @@ export default function (page) {
 
     activate() {
       this._super(...arguments);
-      jumpToElement(document.location.hash.slice(1));
+      DiscourseURL.jumpToElement(document.location.hash.slice(1));
     },
 
     model() {
diff --git a/app/assets/javascripts/discourse/app/lib/url.js b/app/assets/javascripts/discourse/app/lib/url.js
index 4fe557c9cc8..b41e3e5c432 100644
--- a/app/assets/javascripts/discourse/app/lib/url.js
+++ b/app/assets/javascripts/discourse/app/lib/url.js
@@ -73,29 +73,6 @@ let _jumpScheduled = false;
 let _transitioning = false;
 let lockOn = null;
 
-export function jumpToElement(elementId) {
-  if (_jumpScheduled || isEmpty(elementId)) {
-    return;
-  }
-
-  const selector = `#main #${elementId}, a[name=${elementId}]`;
-  _jumpScheduled = true;
-
-  schedule("afterRender", function () {
-    if (lockOn) {
-      lockOn.clearLock();
-    }
-
-    lockOn = new LockOn(selector, {
-      finished() {
-        _jumpScheduled = false;
-        lockOn = null;
-      },
-    });
-    lockOn.lock();
-  });
-}
-
 const DiscourseURL = EmberObject.extend({
   isJumpScheduled() {
     return _transitioning || _jumpScheduled;
@@ -237,8 +214,8 @@ const DiscourseURL = EmberObject.extend({
     // Scroll to the same page, different anchor
     const m = /^#(.+)$/.exec(path);
     if (m) {
-      jumpToElement(m[1]);
-      return this.replaceState(path);
+      this.jumpToElement(m[1]);
+      return;
     }
 
     const oldPath = this.router.currentURL;
@@ -479,9 +456,33 @@ const DiscourseURL = EmberObject.extend({
     transition._discourse_original_url = path;
 
     const promise = transition.promise || transition;
-    promise.then(() => jumpToElement(elementId));
+    promise.then(() => this.jumpToElement(elementId));
+  },
+
+  jumpToElement(elementId) {
+    if (_jumpScheduled || isEmpty(elementId)) {
+      return;
+    }
+
+    const selector = `#main #${elementId}, a[name=${elementId}]`;
+    _jumpScheduled = true;
+
+    schedule("afterRender", function () {
+      if (lockOn) {
+        lockOn.clearLock();
+      }
+
+      lockOn = new LockOn(selector, {
+        finished() {
+          _jumpScheduled = false;
+          lockOn = null;
+        },
+      });
+      lockOn.lock();
+    });
   },
 });
+
 let _urlInstance = DiscourseURL.create();
 
 export function setURLContainer(container) {
diff --git a/app/assets/javascripts/discourse/tests/unit/lib/url-test.js b/app/assets/javascripts/discourse/tests/unit/lib/url-test.js
index e70e65b42ce..632e5435977 100644
--- a/app/assets/javascripts/discourse/tests/unit/lib/url-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/lib/url-test.js
@@ -153,4 +153,13 @@ module("Unit | Utility | url", function () {
       )
     );
   });
+
+  test("anchor handling", async function (assert) {
+    sinon.stub(DiscourseURL, "jumpToElement");
+    DiscourseURL.routeTo("#heading1");
+    assert.ok(
+      DiscourseURL.jumpToElement.calledWith("heading1"),
+      "in-page anchors call jumpToElement"
+    );
+  });
 });