-
Notifications
You must be signed in to change notification settings - Fork 220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🐛 Remove slash duplication in user profile URLs #3009
🐛 Remove slash duplication in user profile URLs #3009
Conversation
Fixed slash duplication in the path component to every user profile by removing trailing slash inside the constant `originalItchio`. The bug is caused by a double slash in the path, for example, `https://itch.io//profile/the-user-name`.
Hello @leafo, I’ve submitted the pull request about "slash duplication in user profile URLs" on Thank you! |
Thanks for the contribution @botamochi0x12 ! I don't see any negative side effects of the changes. If I could review, I would provide a proper one. The constant in question is only used in a handful of places: src\main\butlerd\make-butler-instance.ts:
42 "--address",
43: urls.itchio,
44 "--user-agent",
src\main\reactors\login.ts:
66 widgetParams: {
67: url: recaptchaUrl || urls.itchio + "/captcha",
68 },
176 const epoch = Date.now() * 0.001;
177: const parsed = urlParser.parse(urls.itchio);
178 const opts = { Added slash for subdirectory and parsed URL doesn't need slashes at the end. src\main\reactors\url.ts:
66 wind: "root",
67: url: `${urls.itchio}/profile/${slug}`,
68 }) Added slash for subdirectory. src\main\util\paths.ts:
13 let usersPath = join(app.getPath("userData"), "users");
14: if (urls.itchio !== urls.originalItchio) {
15: usersPath = join(usersPath, fsFriendlyHost(urls.itchio));
16 }
31 if (process.env.WHEN_IN_ROME) {
32: const parsed = urlParser.parse(urls.itchio);
33 dbName = `butler-${parsed.host.replace(/^[A_Za-z\._\-]/g, "_")}.db`; Comparison with itself and parsed URL doesn't need slashes at the end. src\renderer\App\Layout\NonLocalIndicator.tsx:
22 render() {
23: if (urls.itchio === urls.originalItchio) {
24 return null;
28 <IndicatorDiv title="itch is running off a private itch.io instance">
29: {urls.itchio}
30 </IndicatorDiv> Comparison with itself and display only. src\renderer\pages\FeaturedPage.tsx:
12 replace: true,
13: url: urls.itchio,
14 }); Fetch from URL doesn't need slash. src\renderer\pages\BrowserPage\newTabItems.ts:
29 icon: "shuffle",
30: url: urls.itchio + "/randomizer",
31 },
34 icon: "shopping_cart",
35: url: urls.itchio + "/games/on-sale",
36 },
39 icon: "star",
40: url: urls.itchio + "/games/top-sellers",
41 },
44 icon: "fire",
45: url: urls.itchio + "/devlogs",
46 }, Added slash for subdirectory. src\renderer\pages\CollectionPage\index.tsx:
125 // we don't know the slug, the website will redirect to the proper one
126: let url = `${urls.itchio}/c/${collectionId}/hello`;
127 dispatch(actions.openInExternalBrowser({ url })); Added slash for subdirectory. src\renderer\scenes\HubScene\Sidebar\SearchResultsBar\index.tsx:
80 wind: ambientWind(),
81: url: `${urls.itchio}?${encodeURIComponent(this.props.query)}`,
82 }) Query parameters starting after |
Woa thanks for your fast reply @3ter !
I'm glad to hear that you would review my PR. |
@botamochi0x12 But I can't 😉 . I don't have the rights to approve a PR as I'm no member of this repository. I tried to get invested but for me the hurdle was too high (a mix of various factors). I just saw this small change and thought this is small enough to help a little with. Pinging the maintainer was the right thing to do. @alts is also very helpful and I suppose his review could actually be of weight 😃. |
Thank you for your advice, @3ter, and I'm sorry for the delayed reply. |
Thanks for the PR, and sorry for the long wait! This looks great |
Summary
Fixes slash duplication in user profile URLs.
Cause & Impact
The bug arises from a double slash in the path, resulting in URLs like
https://itch.io//profile/the-user-name
.The constant
originalItchio
includes the trailing slash.Considerations
As mentioned in the issue #2975, other paths may also be affected. Introducing a package to construct URLs from bare strings could address this issue more comprehensively.