Skip to content

Conversation

@gary-kim
Copy link
Member

Closes #392
Closes #388

Fix double encoding issue.

@gary-kim gary-kim added bug Something isn't working 3. to review Waiting for reviews labels Feb 10, 2020
@gary-kim gary-kim added this to the Nextcloud 19 milestone Feb 10, 2020
@juliusknorr
Copy link
Member

@gary-kim Thanks for looking into this. Actually it seems that with this opening files in subdirectories is still broken like "/folder with spaces/foo.jpg" as there is also an encodePath call in FileList.js.

Sadly I cannot find any hint on what #357 was supposed to fix. Maybe @ChristophWurst knows, since you have reviewed it?

@gary-kim gary-kim added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 10, 2020
@skjnldsv
Copy link
Member

See nextcloud/photos#122
This is where it began :D

@rullzer
Copy link
Member

rullzer commented Feb 10, 2020

@gary-kim Thanks for looking into this. Actually it seems that with this opening files in subdirectories is still broken like "/folder with spaces/foo.jpg" as there is also an encodePath call in FileList.js.

Are you sure? This works just fine for me.

@rullzer
Copy link
Member

rullzer commented Feb 10, 2020

For the record. I'm fine with basically reverting #357 spaces are more common that ? in filenames.
We should just fix that separately ;)

@gary-kim
Copy link
Member Author

See nextcloud/photos#122
This is where it began :D

Huh, I just tried opening a file with this patch applied that had ? in the name but it still opened fine. I'm gonna check a few more things.

@juliusknorr
Copy link
Member

The path is already properly encoded in the webdav library https://github.com/perry-mitchell/webdav-client/blob/5134ed5dcdda8c7d33f1f3f914904564a52a5935/source/interface/stat.js#L9 so while https://github.com/nextcloud/photos/pull/122/files was needed to fix the encoding of paths in plain axios calls. I'd vote for just reverting #357 then since I cannot find a scenario that was not working before.

@juliusknorr
Copy link
Member

Also having a ? works just fine without #357

@rullzer
Copy link
Member

rullzer commented Feb 11, 2020

Thanks for checking @juliushaertl @gary-kim !
Yes lets just revert #357 then

@gary-kim
Copy link
Member Author

Closing in favor of #395

@gary-kim gary-kim closed this Feb 11, 2020
@gary-kim gary-kim deleted the fix/388/double-encoding-fix branch February 11, 2020 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NC18.0.1 RC1 could not load pictures on webpage Double encoding of the path breaks opening files with spaces

5 participants