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

"Edit page" link on TOC is broken #11994

Closed
2 tasks
visvirial opened this issue Jan 19, 2024 · 9 comments
Closed
2 tasks

"Edit page" link on TOC is broken #11994

visvirial opened this issue Jan 19, 2024 · 9 comments
Assignees
Labels
bug 🐛 Something isn't working high priority This has a high priority

Comments

@visvirial
Copy link
Contributor

Describe the bug

The "Edit page" button link on the top of the TOC is broken.

Currently, it will redirect to e.g.:
https://ethereum.org/github.com/ethereum/ethereum-org-website/tree/dev/public/content/developers/docs/index.md

But it should be:
https://github.com/ethereum/ethereum-org-website/tree/dev/public/content/developers/docs/index.md

instead.

It seems to be the link href is parsed as a relative path, but we should treat it as an absolute path.

To reproduce

  1. Go to somewhere in the "Developers => Docs" page
  2. Click on the "Edit page" button

Expected behavior

It should be redirected to Github, instead of ethereum.org.

Screenshots

No response

Desktop (please complete the following information)

No response

Smartphone (please complete the following information)

No response

Additional context

No response

Would you like to work on this issue?

  • Yes
  • No
@visvirial visvirial added the bug 🐛 Something isn't working label Jan 19, 2024
@github-actions github-actions bot added the needs triage 📥 This issue needs triaged before being worked on label Jan 19, 2024
@dbarabashh
Copy link
Contributor

It seems that this has already been fixed but not deployed to production

@HiroyukiNaito
Copy link
Contributor

If it is not fixed, I would like to work on the issue.

@HiroyukiNaito
Copy link
Contributor

@wackerow
Is it fixed? If not, I doubt it is caused only by SSG. Is there any way to build same as ethereum.org SSG for locating the cause?

@wackerow
Copy link
Member

A patch was put up for this, but it appears there is still a bug with it:

// Inside /src/lib/utils/editPath.ts, we return:

join(EDIT_CONTENT_URL, CONTENT_DIR, relativePath, "index.md")

EDIT_CONTENT_URL has https:// at the beginning of it... we cannot pass this to join() because that function will collapse repeating slashes (/) down to only one, as it's forming a path.

We should use construct a URL with new URL(path, base) instead and return the href value from the resulting object, passing join(CONTENT_DIR, relativePath, "index.md") as the path argument, and EDIT_CONTENT_URL as the base... this will properly parse these into a URL.

ie. (new URL(join(CONTENT_DIR, relativePath, "index.md"), EDIT_CONTENT_URL)).href

@HiroyukiNaito Assigning you for this =)

@HiroyukiNaito
Copy link
Contributor

@wackerow Thanks for the tips! I work on it.

@wackerow
Copy link
Member

wackerow commented Feb 1, 2024

Hey @HiroyukiNaito! Thanks for offering to patch this... Just want to mention that this bug is somewhat important since it's breaking all the edit links in the docs. We were hoping to get this patched ASAP...

Any chance you're able to make this patch in the next few hours? Otherwise I may put up a patch so we can hot-fix the deploy planned for later today. Sorry for the time crunch 🙏

@wackerow wackerow added high priority This has a high priority and removed needs triage 📥 This issue needs triaged before being worked on labels Feb 1, 2024
@wackerow
Copy link
Member

wackerow commented Feb 1, 2024

Hey @HiroyukiNaito, apologies for jumping in on this, but given the nature of this bug we wanted to get a patch up for this ASAP... I put up a PR to fix, but thank you for offering to help!

@wackerow wackerow assigned wackerow and unassigned HiroyukiNaito Feb 1, 2024
corwintines added a commit that referenced this issue Feb 2, 2024
hot-fix: update page edit path logic [Fixes #11994]
@HiroyukiNaito
Copy link
Contributor

HiroyukiNaito commented Feb 2, 2024

@wackerow Sure, thanks for notifying that!

@visvirial
Copy link
Contributor Author

@corwintines @HiroyukiNaito
Thank you for the fix and the deployment.
I could confirm that the links are correct on the production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working high priority This has a high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@visvirial @dbarabashh @HiroyukiNaito @wackerow and others