Skip to content

Conversation

@SegoCode
Copy link
Contributor

Fix the url when the filters are selected, to one that exists

Changes

Context

Fix #1886

Fix the url when the filters are selected, to one that exists
@dscho
Copy link
Member

dscho commented Sep 27, 2024

@SegoCode what do you think, should we also add a commit to redirect gracefully from the now-404ing URLs? Something like this?

diff --git a/content/downloads/guis/_index.html b/content/downloads/guis/_index.html
index 1429c7e91..819c2a463 100644
--- a/content/downloads/guis/_index.html
+++ b/content/downloads/guis/_index.html
@@ -5,6 +5,8 @@ title: "Git - GUI Clients"
 url: /downloads/guis.html
 aliases:
 - /downloads/guis/index.html
+- /download/guis/index.html
+- /download/guis.html
 ---
 
 <div id="main">

@dscho
Copy link
Member

dscho commented Sep 27, 2024

Something like this?

Hmm. That does not quite work, because the ?os=linux part is lost in that redirection. Let me dig deeper.

We just fixed a bug where clicking on an operating system button in
https://git-scm.com/downloads/guis replaced the URL by an incorrect URL,
e.g. https://git-scm.com/download/guis?os=linux instead of
https://git-scm.com/downloads/guis?os=linux (note the singular
"download" in the incorrect URL).

We want to handle such URLs gracefully, though, just in case those links
were already shared.

Therefore we are about to add aliases, but the way aliases are currently
handled loses the `?os=linux` part of the URL, which is rather
important.

Let's copy that part (which is called `window.location.search` in
Javascript) into the new URL in the Javascript part of the redirect
page (which is typically taking precedence over the `<meta
http-equiv="refresh" ...>` tag).

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member

dscho commented Sep 27, 2024

@SegoCode I think I've found it. I have to patch layouts/alias.html where it tries to preserve window.location.hash (i.e. any anchor) so that it also tries to preserve window.location.search. Will push my proposed solution in a moment.

We just fixed incorrect Javascript code that would replace the URL in
the browser window by an incorrect URL when filtering the GUIs by
operating system.

With this here commit, we retroactively handle those incorrect URLs by
redirecting to the correct URLs instead of 404ing.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member

dscho commented Sep 27, 2024

I have to patch layouts/alias.html where it tries to preserve window.location.hash (i.e. any anchor) so that it also tries to preserve window.location.search.

Here is that fix: 40fc16d

And after that, this idea is implemented in f00a03a, which I verified locally to work.

@SegoCode what do you think? Are these two commits okay with you?

@SegoCode
Copy link
Contributor Author

looks good! and if you have tested it in your local and it works as expected, it's OK for me

@dscho dscho merged commit 205db72 into git:gh-pages Sep 27, 2024
1 check passed
@dscho
Copy link
Member

dscho commented Sep 27, 2024

@SegoCode Thank you for noticing, reporting and fixing the bug!

Please note that due to git-scm.com using Cloudflare's caches, the fix may be delayed for up to 4 hours (and more, if your browser cache holds onto /js/application.min.js for some time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The filter url in the GUI Clients section does not exist

2 participants