Scan url with QR scanner #7880
Scan url with QR scanner #7880
Conversation
Jenkins BuildsClick to see older builds (28)
|
|
@krisc thanks for your contribution! maybe @yenda or @flexsurfer can help you with the error function. But I would check if what is returned is similar to a URL, and show error otherwise. |
|
@krisc also, the formatting should be fixed (that's why CI builds are failing). I recommend running |
Oh, thanks! wasn't aware of that command. I get that error every time and didn't know why. |
@yenda or @flexsurfer , is there a function somewhere in status that checks for vaild URLs? Perhaps someone could direct me to the code where URLs in chat messages are rendered as blue and underlined? For now, I'm just checking if the string has a "." in it. |
|
@mandrigin This should be it unless I should be error checking a different way. And FYI, the case where browser selection is canceled and then being able to scan another qr-code depends on code that came from this PR #7745 This PR pretty much has the aforementioned PR merged since it shares so much code. The other PR should probably be merged to upstream first. |
| (fx/merge cofx | ||
| (navigation/navigate-to-clean :home {}) | ||
| (process-qr-code data))) | ||
| (if (clojure.string/includes? data ".") |
There was a problem hiding this comment.
I think you can use something like that to check for URLs:
(defn is-url [url]
(try
(and (js/URL. url) true)
(catch js/Error e false)))There was a problem hiding this comment.
hmm this seems to always return true
There was a problem hiding this comment.
hmm, I tried it in a repl and for me it returns false for, say, “aaa” and true for “http://google.com"...
There was a problem hiding this comment.
hmm I'm using figwheel for status-im, and returns true for anything. I did try in a web repl, and it does return false for something like "aaa". which repl did you use?
There was a problem hiding this comment.
I think this should be handled the same way a URL is handled when tapped on by a user in chat so I would like to see where in the code that renders a chat message as a link. for example, if a message contains "aaaa google.com" then the 'google.com' becomes underlined. I'm having trouble trying to find where this happens :/
There was a problem hiding this comment.
we have a regx-url in constants.cljs to check for urls
|
@mandrigin @yenda I ended up using |
|
Derp, I should have read the code https://github.com/status-im/status-react/blob/64b63d5593cebd7382b6894a71848589f9628e67/src/status_im/extensions/core.cljs#L865 It will always return
If someone could point me to where this happens, that would be great! |
|
@krisc this is done in |
Exactly what I was looking for. Thanks! |
|
Ok, this should be it! |
|
@mandrigin not sure why it's saying "jenkins/prs/android-e2e — This commit cannot be built". I ran |
|
@krisc looks like a random thing, restarted it |
|
@mandrigin BTW is there a way I can get access to the jenkins build logs? When I click on "Details" is says |
|
@krisc I think the access is limited until we audit our build logs for sensitive info... We will get there, it isn't just done yet. |
1) Part of header is not white (Android)
2) After opening URL in native browser can't scan valid URLSteps:
3) Several times I saw crash on Android after scanning valid QRUnfortunately didn't notice any specific steps to reproduce, during testing I scanned ~30 times different URLs and three times I saw this error anter tapping on "Open to Status" when scanning QR with link. Logcat:
4) Ethereum address is considered as a valid link.I didn't expect that wallet address can be scanned with "Scan QR code" in chat. |
|
@churik thanks for uncovering these issues. I'll have more time to look into them next week. |
|
Sorry, I've been busy with Ethereal Hackathon these past few weeks. I'll have more time to get back on this after the weekend. But these issues may be related to some code I wrote for this issue #7745 (comment) |
Pull Request Checklist
|
|
hey @krisc any updates on this? :) |
I was gonna try to resolve issues with #7745 (comment) because it might be related to the issues here. Sorry I'm taking so long. I've been have some issues building and running since we switched to nix. So testing has been really slow for me. |
|
I think I got my nix issues taken care of, but the easiest thing for me to move forward with this issue is to start a new PR. hope that's okay. |
|
yeah that's fine, thanks! |
This PR is for this bounty #5776
It fixes the issue as described, but I left it WIP because I'm not sure if additional checks have to be made or not. For example, if the qr-code returns data
30032621should this show an error message or should it just navigate to whatever that data is in the browser?If it should show an error, do we have a function already in status that checks for valid urls?