Skip to content
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

link-to-file-in-file-history breaks when branches contain slashes #2692

Closed
Jackenmen opened this issue Jan 12, 2020 · 9 comments · Fixed by #2731
Closed

link-to-file-in-file-history breaks when branches contain slashes #2692

Jackenmen opened this issue Jan 12, 2020 · 9 comments · Fixed by #2731
Labels
bug help wanted small Issues that new contributors can pick up

Comments

@Jackenmen
Copy link
Contributor

Bug appears on this page: https://github.com/Cog-Creators/Red-DiscordBot/commits/V3/develop/docs/install_linux_mac.rst

Clicking any of "See object at this point in history" buttons redirects to 404 page.
The issue might be related to the slash in branch name, but I'm not entirely sure.

@Jackenmen Jackenmen added the bug label Jan 12, 2020
@yakov116
Copy link
Member

@jack1142 working fine for me

@Jackenmen
Copy link
Contributor Author

Jackenmen commented Jan 12, 2020

@yakov116 I'm able to reproduce it on clean browser (Firefox) with only Refined GitHub installed.
I'm talking about this button:
image

FYI, this is an example invalid link that button redirects to:
https://github.com/Cog-Creators/Red-DiscordBot/tree/a9d3e271b0fbbe7b0f418cddd0bfa06e3341b926/develop/docs/install_linux_mac.rst

@yakov116
Copy link
Member

yakov116 commented Jan 12, 2020

Linkes to https://github.com/Cog-Creators/Red-DiscordBot/commit/a9d3e271b0fbbe7b0f418cddd0bfa06e3341b926#diff-32cb4e9456317305312fe885c1660614
When I try it.
What version are you on?

Sorry was clicking the wrong button

@yakov116
Copy link
Member

yakov116 commented Jan 12, 2020

@jack1142 ok that button should not be showing unless your looking for at the history of an item.
see https://github.com/jack1142/Red-DiscordBot/commits/b73f95b2b12ad1fa43d4a32ecf14a908e52e2606/docs/install_linux_mac.rst

Doing good today. I cant figure out why

@Jackenmen
Copy link
Contributor Author

Jackenmen commented Jan 12, 2020

@yakov116 hmm, it actually should show on the page I linked in the issue description (as in, it's actually useful to have it there), it just doesn't seem to work properly with any branch that has slashes, I've been able to reproduce this on history page of file on V3/develop and V3/sqlite_driver branches, but not on the history page of file on develop.

Not working example on V3/develop:
https://github.com/Cog-Creators/Red-DiscordBot/commits/V3/develop/docs/install_linux_mac.rst

Not working example on V3/sqlite_driver:
https://github.com/Cog-Creators/Red-DiscordBot/commits/V3/sqlite_driver/docs/framework_bank.rst

Working example on develop:
https://github.com/Cog-Creators/Red-DiscordBot/commits/develop/cogs/customcom.py

@yakov116
Copy link
Member

yakov116 commented Jan 12, 2020

@jack1142 in such a case looks like were not taking out the branch name entirely so it does not work!
Which means any that...
looks like the check is failing
https://github.com/sindresorhus/refined-github/blob/eca2edfb4cc9ab68682aa438ce9bd61258100863/source/features/link-to-file-in-file-history.tsx#L11
It needs to remove the branch name too if it has a slash

See working link
https://github.com/Cog-Creators/Red-DiscordBot/blob/3febc948714b3b1ff01745f492af6e2357a7437c/docs/framework_bank.rst

@yakov116
Copy link
Member

@fregante would document.querySelector(".file-navigation >.breadcrumb").textContent.split('/').splice(1).join('/').trim() be a good idea?
I dont see any other way to find out if the current branch has a /

@fregante
Copy link
Member

Damn branches with slashes, GitHub should really escape them.

I don’t know if your solution works, but the alternative is to get the branch name from another part of the page like we do in other features.

@fregante fregante added small Issues that new contributors can pick up help wanted labels Jan 13, 2020
@fregante fregante changed the title link-to-file-in-file-history links to 404 link-to-file-in-file-history breaks when branches contain slashes Jan 13, 2020
@fregante
Copy link
Member

fregante commented Jan 13, 2020

Or you can also look for other places where the file path is more easily accessible in the DOM. In some feature we use the meta tags in the document head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted small Issues that new contributors can pick up
Development

Successfully merging a pull request may close this issue.

3 participants