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

URL rewritting is not economical (and probably not robust/fast) #904

Open
kelson42 opened this issue Jul 21, 2019 · 9 comments
Open

URL rewritting is not economical (and probably not robust/fast) #904

kelson42 opened this issue Jul 21, 2019 · 9 comments

Comments

@kelson42
Copy link
Collaborator

While relative liniks are perfectly correct, they are not quite as economical as they could be. Relative links on the landing of WVEN page look of like this:

<a title="Europe" href="../../A/Europe">Europe</a>

It would be sufficient for them to be coded like this:

<a title="Europe" href="../Europe">Europe</a>

In other words, the relative links take us two levels up and then back down into the current namespace, which isn't necessary.

I thinks this means that we have custom relative path computation. I wonder why we don't use https://millermedeiros.github.io/mdoc/examples/node_api/doc/path.html#path.relative. This would offer many advantages:

  • Avoid the many bugs we have already detected - and still have open - around this feature
  • Avoid many unit tests around that
  • A solution for sure a lot faster than what we have (and this routine is used a lot)
@ISNIT0
Copy link
Contributor

ISNIT0 commented Jul 22, 2019

Not 1.9/2.0

@ISNIT0 ISNIT0 removed this from the 1.9-maintenance milestone Jul 22, 2019
@kelson42
Copy link
Collaborator Author

kelson42 commented Jul 22, 2019

@ISNIT0 The current code still fails in some cases see #726, even after opening and reopening many time tickets. So why not using the most simple/efficient solution to fix properly things? Why it has not been used until now?

@ISNIT0
Copy link
Contributor

ISNIT0 commented Jul 22, 2019

The current "re-opening" seems to be nothing to do with slash re-writing, and more to do with special namespaces being handled weirdly.

@stale
Copy link

stale bot commented Nov 1, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@kelson42
Copy link
Collaborator Author

kelson42 commented May 9, 2020

@midi To me we should have a function like this: getUrlForZimHtml(contentId, [mime-type], [current]).

@kelson42 kelson42 removed the bug label May 11, 2020
@stale
Copy link

stale bot commented Jul 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot added the stale label Jul 11, 2020
@kelson42 kelson42 modified the milestones: 2.0, 1.13 Nov 14, 2020
@stale stale bot removed the stale label Nov 14, 2020
@stale
Copy link

stale bot commented Feb 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale
Copy link

stale bot commented Jun 16, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot added the stale label Jun 16, 2021
@kelson42 kelson42 modified the milestones: 1.14.0_old, 1.13.0 Jan 20, 2023
@stale stale bot removed stale labels Jan 20, 2023
@kelson42 kelson42 modified the milestones: 1.13.0, 1.14.0 Mar 5, 2023
@stale
Copy link

stale bot commented May 26, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot added the stale label May 26, 2023
@stale stale bot removed the stale label Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants