FIX: Further improvements to `//` handling (#28211)
Followup to f70a65ea02
1. Update a second regex in `routeTo` to avoid stripping domain/protocol from middle of string
2. Update `URL.handleURL` to strip double-slashes in paths, before calling the ember router. This mimics what Ember does on initial page-load
Additional tests are added for both
This commit is contained in:
parent
18ac600352
commit
1b2e0e86f8
|
@ -235,7 +235,7 @@ class DiscourseURL extends EmberObject {
|
||||||
|
|
||||||
const oldPath = this.routerService.currentURL;
|
const oldPath = this.routerService.currentURL;
|
||||||
|
|
||||||
path = path.replace(/(https?\:)?\/\/[^\/]+/, "");
|
path = path.replace(/^(https?\:)?\/\/[^\/]+/, "");
|
||||||
|
|
||||||
// handle prefixes
|
// handle prefixes
|
||||||
if (path.startsWith("/")) {
|
if (path.startsWith("/")) {
|
||||||
|
@ -437,6 +437,10 @@ class DiscourseURL extends EmberObject {
|
||||||
elementId = split[1];
|
elementId = split[1];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Remove multiple consecutive slashes from path. Same as Ember does on initial page load:
|
||||||
|
// https://github.com/emberjs/ember.js/blob/8abcd000ee/packages/%40ember/routing/history-location.ts#L146
|
||||||
|
path = path.replaceAll(/\/\/+/g, "/");
|
||||||
|
|
||||||
const transition = this.routerService.transitionTo(path);
|
const transition = this.routerService.transitionTo(path);
|
||||||
|
|
||||||
transition._discourse_intercepted = true;
|
transition._discourse_intercepted = true;
|
||||||
|
|
|
@ -0,0 +1,19 @@
|
||||||
|
import { currentURL, settled, visit } from "@ember/test-helpers";
|
||||||
|
import { test } from "qunit";
|
||||||
|
import DiscourseURL from "discourse/lib/url";
|
||||||
|
import { acceptance } from "discourse/tests/helpers/qunit-helpers";
|
||||||
|
|
||||||
|
acceptance("DiscourseURL", function () {
|
||||||
|
test("handleURL strips multiple slashes", async function (assert) {
|
||||||
|
await visit("/");
|
||||||
|
|
||||||
|
DiscourseURL.handleURL("/t//280");
|
||||||
|
await settled();
|
||||||
|
|
||||||
|
assert.strictEqual(
|
||||||
|
currentURL(),
|
||||||
|
"/t/internationalization-localization/280"
|
||||||
|
);
|
||||||
|
assert.dom("#topic-title").exists();
|
||||||
|
});
|
||||||
|
});
|
|
@ -141,11 +141,17 @@ module("Unit | Utility | url", function (hooks) {
|
||||||
"it strips the protocol and domain when protocol-less"
|
"it strips the protocol and domain when protocol-less"
|
||||||
);
|
);
|
||||||
|
|
||||||
DiscourseURL.routeTo("https://example.com//foo4");
|
DiscourseURL.routeTo("https://example.com/t//1");
|
||||||
assert.ok(
|
assert.ok(
|
||||||
DiscourseURL.handleURL.calledWith(`//foo4`),
|
DiscourseURL.handleURL.calledWith(`/t//1`),
|
||||||
"it does not strip double-slash in the middle of urls"
|
"it does not strip double-slash in the middle of urls"
|
||||||
);
|
);
|
||||||
|
|
||||||
|
DiscourseURL.routeTo("/t//2");
|
||||||
|
assert.ok(
|
||||||
|
DiscourseURL.handleURL.calledWith(`/t//2`),
|
||||||
|
"it does not strip double-slash in the middle of urls, even without a domain"
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("routeTo does not rewrite routes started with /my", async function (assert) {
|
test("routeTo does not rewrite routes started with /my", async function (assert) {
|
||||||
|
|
Loading…
Reference in New Issue