Fix #3397

[hiiwave]

Description

This is trying to fix #3397.

(Update: Done)
This is working in progress. Since I have little experience building electron app, I haven't tried if this solution really works.

I can try to build it and confirm later.

Issue fixed

#3397

Type of changes

• ⚫ Bug fix (Change that fixed an issue)
• ⚪ Breaking change (Change that can cause existing functionality to change)
• ⚪ Improvement (Change that improves the code. Maybe performance or development improvement)
• ⚪ Feature (Change that adds new functionality)
• ⚪ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

• ⚫ My code follows the project code style
• ⚪ I have written test for my code and it has been tested -> I think it's untestable
• ⚫ All existing tests have been passed
• ⚫ I have attached a screenshot/video to visualize my change if possible -> Please see Cannot open attachment on storage with path containing non-English letter #3397 (comment)

[hiiwave]

(Update: Solved)
(Asking for help on project building)

Hi,

I tried to run the tests for this project (before editing any code) on Windows but failed. As I am not familiar to npm, can anyone advises what happened and how I could fix this here?
The steps to reproduce the problem are as follows:

git clone https://github.com/BoostIO/Boostnote.git
cd Boostnote
npm i
npm run test

It then shows the following log / error:

> [email protected] test C:\Users\hiiwa_000\resources\Boostnote
> npm run ava && npm run jest


> [email protected] ava C:\Users\hiiwa_000\resources\Boostnote
> cross-env NODE_ENV=test ava --serial

C:\Users\hiiwa_000\resources\Boostnote\tests\helpers\setup-browser-env.js:29
window.localStorage = {
                    ^

TypeError: Cannot set property localStorage of #<Window> which has only a getter
    at Object.<anonymous> (C:/Users/hiiwa_000/resources/Boostnote/tests/helpers/setup-browser-env.js:22:1)
    at Module._compile (module.js:652:30)
    at loader (C:\Users\hiiwa_000\resources\Boostnote\node_modules\babel-register\lib\node.js:144:5)
    at Object.require.extensions.(anonymous function) [as .js] (C:\Users\hiiwa_000\resources\Boostnote\node_modules\babel-register\lib\node.js:154:7)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)
    at forEach.x (C:\Users\hiiwa_000\resources\Boostnote\node_modules\ava\lib\test-worker.js:34:3)

...

  32 exceptions

  × tests\dataApi\copyFile-test.js exited with a non-zero exit code: 1

...

All the tests failed with the exactly same reason.

I also tried npm run compile before npm run test, but it still does not work. If I run npm run start or npm run dev, I can get electron open, but the app freezes with the spinner spinning forever.

I guessed it's a basic setup issue but I have no idea how to go on. Appreciate for any help.

[Flexo013]

I think you might need to use yarn, but I'm not sure about that. @Rokt33r do you know what can help?

[ZeroX-DG]

You can always check out the job log in the travis build to find out what command was executed for running test:
https://travis-ci.org/BoostIO/Boostnote/builds/624286760

[hiiwave]

Thanks for helping. I have finished this PR and checked that it fixed #3397 under Windows. Awaiting review.

[arcturus140]

the Link should always be encoded on creation and always decoded for onclick. Otherwise it could cause problems with edge cases when a non-encoded file looks like encoded. For example, if you have two files in the same directory:

• %E6%B8%AC%E8%A9%A6.txt
• %25E6%25B8%25AC%25E8%25A9%25A6.txt

[hiiwave]

As it seems the original behavior was fine under mac os, and the issue seems to be Windows-only, I made it conservative that it prioritized original behavior. If you can confirm that "always decoding" works fine under each os, I'm happy to change to that case.

[arcturus140]

@hiiwave would you consider testing it under Windows with always decoding? I am currently testing it on Linux.

[hiiwave]

Always decoding behaves exactly as this implementation under Windows, because shell.openExternal(href) does not work thus it will always do shell.openExternal(decodeURI(href)

[arcturus140]

I can confirm that "always decoding" works fine under Linux.

[arcturus140]

@hiiwave ok, I was assuming edge cases on Windows that make shell.openExternal(href) succeed. If it doesn't then it is ok.

If you create a storage location in the folder %20 both raw and decoded will fail because it must be encoded:

shell.openExternal(encodeURI(href)

works in this case. I don't know where href is stored but it must be encoded on creation.

[hiiwave]

As #3397 (comment):

IMHO, users should not create a folder named %20 in any case. I don't think anyone will ever do it as well.

[ZeroX-DG]

@Rokt33r can you confirm this work on Windows as well?

[Rokt33r]

Will do.

[Rokt33r]

Confirmed it works. @hiiwave Thanks for the contribution!