DEV: Use Ember router to determine currentURL

This means that our DiscourseURL logic will work consistently in tests, where `window.location` doesn't get updated.

To make it work properly, our `replaceState` implementation needed to be updated so that it writes the new URL to Ember's router, rather than bypassing the router and going straight to the `location` API.

A couple of tests needed updating following this fix:
- the composer-test was asserting that the new reply should be missing from the DOM... when really it **should** be in the DOM, and this fix to the test environment makes it so
- the topic-test was making a fake topic fixture based on the data from a topic with a different id. This was causing the topic route to get confused, and 'fix' the currentURL. This commit updates it to use a fixture with consistent data.

This commit also removes the feature detection of `window.history`. It's feature-detected within `discourse-location`. Plus, we don't support any browsers without it.
This commit is contained in:
David Taylor 2022-01-11 18:43:13 +00:00
parent dd3ed27930
commit 78c6fc6e43
4 changed files with 17 additions and 22 deletions

View File

@ -178,22 +178,15 @@ const DiscourseURL = EmberObject.extend({
});
},
// Browser aware replaceState. Will only be invoked if the browser supports it.
replaceState(path) {
if (
window.history &&
window.history.pushState &&
window.history.replaceState &&
window.location.pathname !== path
) {
if (this.router.currentURL !== path) {
// Always use replaceState in the next runloop to prevent weird routes changing
// while URLs are loading. For example, while a topic loads it sets `currentPost`
// which triggers a replaceState even though the topic hasn't fully loaded yet!
next(() => {
const location = this.get("router.location");
if (location && location.replaceURL) {
location.replaceURL(path);
}
// Using the private `_routerMicrolib` is not ideal, but Ember doesn't provide
// any other way for us to do `history.replaceState` without a full transition
this.router._routerMicrolib.replaceURL(path);
});
}
},
@ -249,7 +242,8 @@ const DiscourseURL = EmberObject.extend({
return this.replaceState(path);
}
const oldPath = `${window.location.pathname}${window.location.search}`;
const oldPath = this.router.currentURL;
path = path.replace(/(https?\:)?\/\/[^\/]+/, "");
// Rewrite /my/* urls

View File

@ -202,7 +202,7 @@ acceptance("Composer", function (needs) {
await click("#reply-control button.create");
assert.strictEqual(
queryAll(".cooked:last p").text(),
"If you use gettext format you could leverage Launchpad 13 translations and the community behind it."
"this is the content of my reply"
);
});

View File

@ -586,19 +586,14 @@ acceptance("Navigating between topics", function (needs) {
const firstPost = topicResponse.post_stream.posts[0];
firstPost.cooked += `\n<a class='same-topic-slugless' href='/t/280'>Link 1</a>`;
firstPost.cooked += `\n<a class='same-topic-slugless-post' href='/t/280/3'>Link 2</a>`;
firstPost.cooked += `\n<a class='diff-topic-slugless' href='/t/281'>Link 3</a>`;
firstPost.cooked += `\n<a class='diff-topic-slugless-post' href='/t/281/3'>Link 3</a>`;
firstPost.cooked += `\n<a class='diff-topic-slugless' href='/t/28830'>Link 3</a>`;
firstPost.cooked += `\n<a class='diff-topic-slugless-post' href='/t/28830/1'>Link 4</a>`;
firstPost.cooked += `\n<a class='by-post-id' href='/p/${firstPost.id}'>Link to Post</a>`;
server.get("/t/280.json", () => helper.response(topicResponse));
server.get("/t/280/:post_number.json", () =>
helper.response(topicResponse)
);
server.get("/t/281.json", () => helper.response(topicResponse));
server.get("/t/281/:post_number.json", () =>
helper.response(topicResponse)
);
});
test("clicking slug-less URLs within the same topic", async function (assert) {
@ -613,10 +608,11 @@ acceptance("Navigating between topics", function (needs) {
test("clicking slug-less URLs to a different topic", async function (assert) {
await visit("/t/-/280");
await click("a.diff-topic-slugless");
assert.ok(currentURL().includes("/281"));
assert.ok(currentURL().includes("/28830"));
await visit("/t/-/280");
await click("a.diff-topic-slugless-post");
assert.ok(currentURL().includes("/281"));
assert.ok(currentURL().includes("/28830"));
});
test("clicking post URLs", async function (assert) {

View File

@ -77,6 +77,11 @@ module("Unit | Utility | url", function () {
logIn();
const user = User.current();
sinon.stub(DiscourseURL, "router").get(() => {
return {
currentURL: "/forum",
};
});
sinon.stub(DiscourseURL, "handleURL");
DiscourseURL.routeTo("/my/messages");
assert.ok(