vladimiry / ElectronMail

Unofficial ProtonMail Desktop App
GNU General Public License v3.0
1.49k stars 96 forks source link

Proton drive sharing url, incorrect format #365

Closed CodeCracker-oss closed 3 years ago

CodeCracker-oss commented 3 years ago

Hello,

When i attempted to share a file via ProtonDrive, the sharing url format is something like: webclient1://mail.protonmail.com/urls/code

when this put in address bar of browser, obviously doesn't understand it, omitting the webclient1:// brings to a protonmail user logjn area, which isnt correct as the recipent shouldn't need an account. I attempted this sharing directly from drive.protonmail.com and realized the format should be: https://drive.protonmail.com/urls/code

This might confuse some people if it appears different in electronmail. Im not exactly sure what the webclient part even supposed to resemble.

So it doesn't appear to be an upstream issue as it works as expected from protonmail directly.

Can you look into this?

Im running a self-compiled appimage, on Kubuntu 20.10. AppImage was compiled today, from latest source as of today. First time actually testing anything from proton drive.

Thank you

vladimiry commented 3 years ago

It's expected to work the way you described since the proton clients packed into the app get loaded on the custom schemes looking like webclientN (so not https scheme).

I made the app to have proton-drive client packed same as the mail-related clients which helps with early issues detection (you just detected one). I believe I should be able to fix the issue but only having access to the drive service which is not the case yet.

Btw you have the drive link listed in the dropdown menu so you can navigate there?

CodeCracker-oss commented 3 years ago

The drive link as in the one you use to share the file, then yea. Its just that I have to change the first part of it. Do you think you may be able to place a message/notice near that generated link to let others know, at least for the time being until you are able to better address the issue?

vladimiry commented 3 years ago

As I highlighted above, I'm not going to start playing around the drive until I have access to this service. I might want to stop shipping the drive web client in the app for now to prevent possible confusion.

CodeCracker-oss commented 3 years ago

Oh, ok. I didn't know you didn't have access to it, as in because of its current subscription requirement. It would be up to you if you stopped proton-drive for now, but it is useful to be part of it as it does work great, so even a mention in README that as you dont have access to it, you can't, or won't provide support for it, until you do so you can test it.

Anyway, thank you. I'll let you decide wether to close the issue or not.

vladimiry commented 3 years ago

If I got it right, the workaround is to manually replace webclientN-like URL scheme with https.

CodeCracker-oss commented 3 years ago

Yes, ensureing that the mail.protonmail.com appears as drive.protonmail.com instead as well. URL for a shared file would appear as https://drive.protonmail.com/urls/ALSLQKQQOEPDJ

vladimiry commented 3 years ago

Im running a self-compiled appimage

Can you assembly the package from the wip branch and test it then? The 8cd3d7d93f5d8b183171d5f151c4150711c51316 commit might fix the issue but I can't verify myself since have no access to the drive service.

Btw, are you affected by #377?

vladimiry commented 3 years ago

Can you assembly the package from the wip branch and test it then?

The build command: yarn --pure-lockfile && yarn app:dist && npm run electron-builder:dist:linux:appimage (in the master branch the call still listed as yarn electron-builder:dist:linux:appimage which is not the case anymore).

vladimiry commented 3 years ago

@edmundlaugasson, if you have access to the Drive service, can you verify that the original issue has been resolved in the v4.11.0 release?

CodeCracker-oss commented 3 years ago

@edmundlaugasson, if you have access to the Drive service, can you verify that the original issue has been resolved in the v4.11.0 release?

I verified in 4.11.0 (self-comiled appimage release) that the shared urls now work and display as they do on ProtonDrive's official website, so the main issue seems to be resolved.

There is an error now that appears during every launch of ProtonDrive that may be releated to this change, but there doesn't appear to be any noticiable downside from it other than the annoying popup. Error message is below:

Unhandled Error TypeError: Invalid URL protocol at default_1.parse (/tmp/.mount_ElectrdUIhiM/resources/app.asar/node_modules/@cliqz/url-parser/build/cjs/lib/immutable-url.js:420:23) at new default_1 (/tmp/.mount_ElectrdUIhiM/resources/app.asar/node_modules/@cliqz/url-parser/build/cjs/lib/immutable-url.js:54:14) at new default_1 (/tmp/.mount_ElectrdUIhiM/resources/app.asar/node_modules/@cliqz/url-parser/build/cjs/lib/url.js:41:20) at Object.depersonalizeProtonApiUrl (/tmp/.mount_ElectrdUIhiM/resources/app.asar/app/electron-main.js:7452:10) at replacer (/tmp/.mount_ElectrdUIhiM/resources/app.asar/app/electron-main.js:7469:41) at String.replace () at Object.result [as depersonalizeLoggedUrlsInString] (/tmp/.mount_ElectrdUIhiM/resources/app.asar/app/electron-main.js:7470:37) at Object. (/tmp/.mount_ElectrdUIhiM/resources/app.asar/app/electron-main.js:5499:147)

vladimiry commented 3 years ago

There is an error now that appears during every launch of ProtonDrive

Resolved in #386.