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

Issue with QR code in the desktop app #4756

Closed
VDAODAO opened this issue Mar 1, 2021 · 21 comments · Fixed by #4786
Closed

Issue with QR code in the desktop app #4756

VDAODAO opened this issue Mar 1, 2021 · 21 comments · Fixed by #4786
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability.

Comments

@VDAODAO
Copy link

VDAODAO commented Mar 1, 2021

Hi !

Whereas I have no issues when I try to "add via QR code" in the in-browser app, I can't get it to work in the desktop app. It gives me the following error : "Uncaught error. Something went wrong with the query and rendering of this component. Failed to construct 'Worker': Access to the script at 'blob:file:///a61f0d6d-b238-40b5-a1f3-15e899dd8af3' is denied by the document's Content Security Policy."

I’m on macOS Catalina 10.15.7, version of the app is 0.81.1. I also tried it on another macOS computer, same issue.

Capture d’écran 2021-03-01 à 18 21 12

Any idea on what to do ?

Thanks for the great work here !

@jacogr jacogr added @apps-electron Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability. labels Mar 1, 2021
@jacogr
Copy link
Member

jacogr commented Mar 1, 2021

Weird. Sadly no workaround for this type of issue. This is definitely an overly-restrictive config somewhere in the desktop security that disallows it.

Will see how and why it does it.

Earliest release with a solution I can promise is next week Monday as part of the next release cycle for the desktop client - which is not great, hence just setting expectations up-front.

@VDAODAO
Copy link
Author

VDAODAO commented Mar 1, 2021

Thanks a lot for looking at it !

@jacogr
Copy link
Member

jacogr commented Mar 8, 2021

No crashes as of 0.83.1... but the Electron app cannot get the camera permissions, at least not for me on Mac OS. So still broken in my case.

More work required - https://www.electronjs.org/docs/api/system-preferences#systempreferencesaskformediaaccessmediatype-macos

@kianenigma
Copy link

Confirming the same. Not possible to get camera access on Mac
Screenshot 2021-03-13 at 07 59 02

@heisler98
Copy link

I'm leaving this bit of info here:

I can get the camera scan to work if I route app launch through a shell...e.g. run /Applications/Polkadot-JS\ Apps.app/Contents/MacOS/Polkadot-JS\ Apps in iTerm. When I add an account via QR, macOS will ask me if I want to let iTerm access the camera.

Usually this happens when the host app isn't properly asking macOS for camera permission, which is a common problem. Apple has extensive documentation on how to properly request auth for media capture, if it's of any use when using Electron.

@wachulski wachulski self-assigned this Jun 11, 2021
@wachulski
Copy link
Contributor

Here are my quick findings:
The app uses @polkadot/react-qr which in turns employs react-qr-reader by JodusNodus author, who states the following:
https://github.com/JodusNodus/react-qr-reader#known-issues

Server side rendering won't work so only require the component when rendering in a browser environment.
Due to browser implementations the camera can only be accessed over https or localhost.
In Firefox a prompt will be shown to the user asking which camera to use, so facingMode will not affect it.
On IOS 11 it is only supported on Safari and not on Chrome or Firefox due to Apple making the API not available to 3rd party browsers.

There is a similar issue in there:
JodusNodus/react-qr-reader#36 (comment) which states:

The module uses many things that are browser specific. I'm not familiar with next.js but you should be able to avoid rendering it server side.

My conclusion:
Not supported unless we replace the reader facility in @polkadot/react-qr

@wachulski wachulski removed their assignment Jun 11, 2021
@wachulski
Copy link
Contributor

When I debug the electron app with https://github.com/bytedance/debugtron then it works as in the browser (macOS prompts me if I want to allow Debugron access my video camera, I agree and it works then).

@jacogr
Copy link
Member

jacogr commented Jun 11, 2021

Have we tried this - https://www.electronjs.org/docs/api/system-preferences#systempreferencesaskformediaaccessmediatype-macos to get the same effect? (Not sure how that translates to other environments...)

@wachulski
Copy link
Contributor

ah, I got your point now @jacogr: attempting to ask for permissions upfront so then the QR reading library won't hit any obstacle.

@jacogr
Copy link
Member

jacogr commented Jun 11, 2021

No idea, just spitballing. But basically since it really doesn't seem to be a file issue (you got it to work), it is permissions only. Either way, we don't ask for it, so really should...

@wachulski
Copy link
Contributor

Good news! Mere addition of NSCameraUsageDescription via electron-builder.yml to Info.plist helped. At least it seems so. Will test a bit more and create a PR.

@jacogr
Copy link
Member

jacogr commented Jun 12, 2021

Closed in #5569 - will be in Monday's release.

@jacogr jacogr closed this as completed Jun 12, 2021
@wachulski
Copy link
Contributor

I doesn't work in 93.1 either. Looking into it now. I suspect macOS detects its origin as github.com download and rejects the app asking for permission.

@jacogr jacogr reopened this Jun 15, 2021
@wachulski
Copy link
Contributor

Comparing the contents of Info.plist files (locally built which works and release download which does not) gives no diff except:

  1. version
<key>CFBundleShortVersionString</key>
<string>0.93.1</string>
<key>CFBundleVersion</key>
<string>0.93.1</string>
  1. integrity
<key>AsarIntegrity</key>
<string>{&#34;checksums&#34;:{&#34;app.asar&#34;:&#34;SRplUtvkiTlCFYgN3o9WtRJ1G6BYV14VBPKQn17be/iMCAzFjknbffeN27cJrZnYwMiGfangTtVl+1tqjGxk9Q==&#34;}}</string>
    

which is as expected and does not explain the behaviour difference

@wachulski
Copy link
Contributor

Also: the downloaded release DMG has the following attributes:

com.apple.diskimages.fsck
com.apple.diskimages.recentcksum
com.apple.macl
com.apple.metadata:kMDItemWhereFroms
com.apple.quarantine

When I remove all, it still does not help - the released app does not ask to use the camera permission whereas the locally built release package does.

@wachulski
Copy link
Contributor

I'm running out of ideas.

When I debug the app in Debugtron, it works as it is Debugstron who asks for camera permission.

@jacogr
Copy link
Member

jacogr commented Jun 15, 2021

Believe we need this as well - <key>com.apple.security.device.camera</key>

So if we add that, I guess let's add a CHANGELOG entry for the version and drop the check in workflow - that way we can get an intermediate build via CI to check since weirdly, local and CI don't match.

@wachulski
Copy link
Contributor

I could bet all my dev-network tokens it won't help, but let's try it out :D

@jacogr
Copy link
Member

jacogr commented Jun 15, 2021

The above keep on popping up in comments that is linked from electron on the same "Mac doesn't ask for permission" issues... Really clueless, since locally should be the same as CI. Which makes things a huge PITA.

@jacogr
Copy link
Member

jacogr commented Oct 23, 2021

Verified as working on my machine in 0.97.1 (so #5604 seems to have been the fix indeed)

@jacogr jacogr closed this as completed Oct 23, 2021
@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Oct 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Tracks issues causing errors or unintended behavior, critical to fix for reliability.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants