Wrap URL string in quotes to avoid escaping issues #37020
Conversation
This ensures that the browser will expect that single-quotes in the url string are not the end of the string, but part of the url. Fixes nextcloud#31224 Signed-off-by: Colin Alworth <colin@colinalworth.com>
szaimen
requested review from
skjnldsv,
Pytal,
a team and
szaimen
and removed request for
a team
|
Should I also modify the filepicker.html file to include these quotes, even though that part of the template is replaced by this JS? |
Signed-off-by: Colin Alworth <colin@colinalworth.com>
Sure :) |
Signed-off-by: Colin Alworth <colin@colinalworth.com>
|
Just my luck to have a major outage in GHA when I try to make my first PR to this project..
|
|
Anything else I can/should do to help this along? |
|
Yes, please rebase on top of master, run npm run build and commit all files here. Thanks! |
|
I'm afraid I can't follow the setup instructions locally (I'm not going to be giving |
|
Yes, there is a command available that would do this automatically. Unfortunately it does not work for forks. However did you already try npm ci after the rebase and the npm run build? I unfortunately forgot to mention this. |
|
For what its worth, I created the codespace on the main repo, and only created a fork after I wasn't able to build (and manually edited the already-minified js file to test my fix directly). I'll rebase and npm ci, run build and post any updates. |
Hi @niloc132 any update on this? :) |
|
Closing as 27 and 28 got a new picker |
Pull request was closed
This ensures that the browser will expect that single-quotes in the url string are not the end of the string, but part of the url.
Fixes #31224
Signed-off-by: Colin Alworth colin@colinalworth.com
Summary
The "file picker" dialog was previously not able to display previews for files if there was a
'character in their path. This fix wraps the string passed to the cssurl()function with double-quotes, since URLs must escape double quotes (so additional quoting won't be required, whereas if the wrapping was done with a single-quote, then additional single-quotes in the URL would need to be further escaped). This is the simplest fix to ensure that properly-encoded URLs will be recognized by the browser.TODO
N/A
Checklist
Before:
After:
There are some other notes that js should be built and checked in, but it isn't clear to me if CI should do that. The current settings for Github CodeSpaces cannot run the
make dev-setupcommand in its present configuration, and I haven't figured out the precise combination that should build without changes at current HEAD, so any change I make appears to modify every file in dist/.