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

encoding of resources in routes #988

Closed
exalate-issue-sync bot opened this issue Nov 30, 2020 · 8 comments
Closed

encoding of resources in routes #988

exalate-issue-sync bot opened this issue Nov 30, 2020 · 8 comments

Comments

@exalate-issue-sync
Copy link

exalate-issue-sync bot commented Nov 30, 2020

Story

As a user without URL encoding knowledge I want to understand the folder path in the URL so that I do not get confused.

Original issue

When navigating into a folder in the oC-Web files app, the path is reflected in the route - which is a good thing (bookmarks, page reloads, etc). But there are two kind of ugly things:

  1. the full path is urlencoded - which is required for various reasons, e.g. spaces or special characters. But the forward slashes also get encoded to %2F, instead of staying forward slahes. It would look much nicer and sane if they could stay forward slashes instead.
  2. the urlencoded path always contains a leading slash, which also makes the url look broken (.../list/%2Ftest, where /%2F is actually two forward slahes. If we decide to do (1) we should also do (2).
@exalate-issue-sync
Copy link
Author

exalate-issue-sync bot commented Dec 8, 2020

Lukas Hirt commented: I am having two issues when trying to make this happen:

  1. Seems that router does the encoding on it's own even though it shouldn't (see Disabling URI encoding vuejs/vue-router#787 (comment)) since we are passing the path without encoding it. Maybe it could be related to not using the history mode (Disabling URI encoding vuejs/vue-router#787 (comment))?
  2. When trying to navigate into some subfolder via typing the path in the URL e.g. /files/list/Documents/New%20folder, it doens't load the correct route. I think this is because vue-router doesn't consider this as one parameter as soon as there is non-encoded slash but as another route instead.

@exalate-issue-sync
Copy link
Author

exalate-issue-sync bot commented Dec 14, 2020

Florian Schade commented: vue router uses, https://github.com/pillarjs/path-to-regexp. the only way to get it in vue is:
1.) switch to vue-router next, switch to vue 3
2.) monkey patch router

i did 2.) for now. it can be viewed and tested here: owncloud/web@bc960e6

but it don't feel good to patch this. not sure how i should think about it. The problem if we won't do anything and let it be as it is right now we could break bookmarks in future version.

URLS now:
https://localhost:9200/#/files/list/
https://localhost:9200/#/files/list/1/2
https://localhost:9200/#/mediaviewer/files-list/1/2/images.jpg
...

readings:
https://stackoverflow.com/questions/56852432/dynamic-router-link-changes-slash-to-2f
vuejs/vue-router#3350
vuejs/vue-router#2953
...

we could use arays as params but then we need to rewrite every single link creation and $store.push

[~bkulmann] CC

@exalate-issue-sync exalate-issue-sync bot changed the title [oC-Web] files list path contains leading slash in route [oC-Web] encoding of resources in routes Dec 14, 2020
@exalate-issue-sync exalate-issue-sync bot removed the bug label Dec 14, 2020
@kulmann
Copy link
Member

kulmann commented Feb 12, 2021

Duplicate: owncloud/web#4595

@exalate-issue-sync exalate-issue-sync bot changed the title [oC-Web] encoding of resources in routes encoding of resources in routes Feb 15, 2021
@exalate-issue-sync exalate-issue-sync bot added the Type:Story User Story label Feb 16, 2021
@exalate-issue-sync
Copy link
Author

Lukas Hirt commented: I'm thinking now if we should also discuss history mode when implementing this. Reasoning: if it is important for CERN because they are using the URLs and would prefer them not to change - having an agreement on should the URL contain # or not could result in change later. So maybe it would be worth to tackle this at the same time so we do one change of URL for both cases.

@exalate-issue-sync
Copy link
Author

Benedikt Kulmann commented: Makes sense. But that should be a separate story...

@settings settings bot removed the p3-medium label Apr 7, 2021
@stale
Copy link

stale bot commented Jun 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status:Stale label Jun 6, 2021
@kulmann
Copy link
Member

kulmann commented Jun 7, 2021

Still relevant

@stale stale bot removed the Status:Stale label Jun 7, 2021
@stale
Copy link

stale bot commented Aug 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 10 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status:Stale label Aug 6, 2021
@stale stale bot closed this as completed Aug 16, 2021
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.

1 participant