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

Remove encodeUri from squirrel-windows packaging #2558

Closed
wants to merge 2 commits into from
Closed

Remove encodeUri from squirrel-windows packaging #2558

wants to merge 2 commits into from

Conversation

rkistner
Copy link
Contributor

@rkistner rkistner commented Feb 6, 2018

Since Squirrel.Windows 1.7.x, Squirrel doesn't do URI encoding or decoding of paths anymore. Having encodeUri in here leaves the filenames in the escaped form after installation, which causes some issues.

Fixes #2557.

Since Squirrel.Windows 1.7.x, Squirrel doesn't do URI encoding or decoding of pathnames anymore. Having encodeUri in here leaves the filenames in the escaped form after installation, which causes some issues.
@develar
Copy link
Member

develar commented Feb 6, 2018

Thanks a lot. Have you build your app with this change?

@rkistner
Copy link
Contributor Author

rkistner commented Feb 6, 2018

For my app I made this change directly to the built JS file (it's a lot more effort to use a forked repo).

Initial testing looks good, but I still have to test what happens when upgrading from a very old version (where Squirrel still does the unescaping) to the new build.

@develar
Copy link
Member

develar commented Feb 6, 2018

Ignore CI failure — I will fix snapshots.

@rkistner
Copy link
Contributor Author

rkistner commented Feb 6, 2018

I've confirmed that this works for my case for:

  • A fresh install.
  • Upgrading from an electron-builder 19.49.0 build.
  • Upgrading from an electron-builder 5.23.2 build.

In all those cases the exe is correctly installed as My Application.exe (not My%20Application.exe), and electron.autoUpdater.quitAndInstall() worked as expected.

However, I only tested the case with a space in the name - I did not test any non-ASCII characters (I'm not that familiar with how the encoding is supposed to work in those cases).

@develar
Copy link
Member

develar commented Feb 6, 2018

Could you please set product name to 真实光影V4低配 and test setup?

@rkistner
Copy link
Contributor Author

rkistner commented Feb 6, 2018

That does seem to work, both for clean install and on update:

image

The MSI installer does fail to build with a name like that, but I believe that's an unrelated limitation (same happens without this change).

@develar
Copy link
Member

develar commented Feb 6, 2018

Thanks. Fix will be applied and released tomorrow evening CET.

@develar develar closed this in 7a37cef Feb 7, 2018
@rkistner rkistner deleted the patch-1 branch February 8, 2018 10:34
@rkistner
Copy link
Contributor Author

rkistner commented Feb 8, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants