Open fercarcedo opened 4 years ago
Initial test seems to work!
One big issue though, the camera doesn't turn off after the scan completes. I believe this might be related: https://github.com/jbialobr/JsQRScanner/issues/23
Initial test seems to work!
One big issue though, the camera doesn't turn off after the scan completes. I believe this might be related: jbialobr/JsQRScanner#23
I'm going to take a look at it, thank you!
Now the camera seems to be closing as expected. @treeder could you check again?
Yep, shuts camera down now. 👍
Great job, thank you. Unfortunately I'm currently working in a long term project so the review process will take a bit more time. I'll review this PR asap.
Don't worry. Thank you!
FYI, to test this, use this in pubspec.yaml:
barcode_scan:
barcode_scan_web:
git:
url: git://github.com/fercarcedo/flutter_barcode_reader.git
ref: web-support
path: flutter_barcode_reader_web/
Something seems to be going wrong when you make a production build, it can't find the qrcode scanner js file I believe, getting a 404 page instead.
I'm guessing these assets needs to be defined in your pubspec.yaml @fercarcedo : https://flutter.dev/docs/development/ui/assets-and-images#bundling-of-package-assets
They aren't in the build/web folder after a flutter build web
.
I'm guessing these assets needs to be defined in your pubspec.yaml @fercarcedo : https://flutter.dev/docs/development/ui/assets-and-images#bundling-of-package-assets
They aren't in the build/web folder after a
flutter build web
.
I'm going to try again later today, but yesterday I tried placing the files in an asset folder and referencing them in pubspec.yaml (assets/jsqrscanner.nocache.js) and also by leaving them where they are now (inside lib/assets) and in pubspec.yaml registering them with packages/barcode_scan_web/assets/jsqrscanner.nocache.js , but both approaches failed.
Thank you for taking time to investigate
An update on this: I have managed to solve this issue. Turns out that, in addition to having to register the assets in pubspec.yaml, you need to reference them from HTML as assets/packages/package_name/assets (I was using packages/package_name/assets, without the leading assets/, which was somehow working fine on debug builds).
But now there is a different problem which I'll take more time to investigate: the jsqrscanner library reports an error, but only when working on release mode
@fercarcedo Not trying to tell you how to work here, but would you mind not force-pushing to this pull request? Makes it hard to figure out what you changed (and less important, breaks builds that use those lost commits). They can (and most likely will) squash and merge or rebase and merge your pull request anyways.
The asset thing I found weird too, I'm not sure if it's just for flutter web, but it doesn't seem to match the documentation. If you put them in the assets directory, then I think you get what happened to you, it'll add a second assets folder in the build/web directory like this:
I found I had to do the following to make it feel right and get images to show up:
Create an images
folder in the root directory, alongside pubspec.yaml, then in pubspec.yaml:
assets:
- images/
- images/logos/
Then to use them:
image: AssetImage('images/logos/abc.png'),
🤷♂
@devtronic I'm fairly new to flutter, but is embedding the JS rather than linking to a CDN the recommended way to do it for these web components? I couldn't find any discussions on this topic.
I'm getting an error in dev mode now too:
Unfortunately can't help debugging because I don't know what changed since the working commits are gone.
That's weird, I didn't encounter this issue in dev mode, but if you want you can try again with the new commit. I've replaced the minified version of JsQRScanner I was using previously with the unminified one, in order to be able to see the errors more crearly. The issue you're facing is weird, since I'm using allowInterop in order to expose the Dart function scannerReady
to JS.
Regarding force-pushing, I was performing them since they were simple additions and I thought no one was reviewing the commits just yet. Since you are looking at it now, I'll make new commits from now on :)
I've opened an issue on the JsQRScanner repo because I'm starting to think it may be a problem with the library itself (worst case scenario I'll simply use another library and update this PR with it). jbialobr/JsQRScanner#24
Nope, not working:
This all worked fine yesterday though, just not in production. Maybe something else you changed is causing these issues? Can you rollback to when it worked, then start from there again?
Unfortunately, I can't reproduce the issue. I'm trying it with the same code in pubspec.yaml you mentioned:
barcode_scan:
barcode_scan_web:
git:
url: git://github.com/fercarcedo/flutter_barcode_reader.git
ref: web-support
path: flutter_barcode_reader_web/
And everything works fine in debug mode. The only thing I changed after it worked for you was to move the assets files and reference them in pubspec.yaml (but it shouldn't be causing this issue, since given your stacktrace it's finding the library correctly, but it seems it isn't exposing the dart function correctly, although the line that exposes it it's intact). That's weird.
Could you try cleaning up your browser cache in order to see if it makes a difference?
I'm actually hardcoding the commits now:
barcode_scan_web:
git:
url: git://github.com/fercarcedo/flutter_barcode_reader.git
ref: 44841ca3db925fe4136778d9b65e3275094e54f8
path: flutter_barcode_reader_web/
Can you try that and see if it still works? Wonder if you're still using an older version.
I just tried it and it still works. Also I looked into the build/web folder to see the bundled assets inside assets/packages/barcode_scan_web and I verified that the jsqrscanner.nocache.js file is the unminified one, so it appears to be using the latest version. Also, if you look at the commit you can see right there on the dart file the call to allowInterop, so this error seems weird...
I did a flutter clean
and a complete rebuild and still getting the most recent error above with the allowInterop
. 🤷♂
Any chance you still have your history from when it was working?
I put the cdn source back in to ensure it's not the local assets, but got the same allowInterop issue. Then I had to change line 87 to this to get past the allowInterop issue:
_scanner = JsQRScanner(allowInterop(onQRCodeScanned), allowInterop(provideVideo));
But now I'm getting this from the scan:
There has to be something else that changed other than the asset locations.
I'll take a look at it, I'll keep you updated
I've updated the PR and now it's working for me both in debug and release versions (tested on both Chrome and Firefox on the desktop and Chrome and Firefox Preview on Android). Seems that the interop bug you were seeing and the release error were related. @treeder could you check again?
Looks like it works! 👍
Thanks.
I had a friend try this on their iPhone with both Safari and Chrome and it doesn't seem to work properly. I haven't done any investigation, but the same code and everything works fine on Chrome on Android and my desktop Chrome. When he clicks the Scan button, it throws a FormatException immediately:
This error seems to indicate that navigator.getUserMedia is not supported by the browser.
AFAIK it's the only way to access the camera from the browser. I know some old iOS versions don't support this API on Safari, but I think I read that newer ones do (not sure about Chrome though).
Do you know if this iPhone running iOS 13 or an older version?
See this table from caniuse for reference: https://caniuse.com/#search=getusermedia
Although that being said I think we should provide a better error message if the browser doesn't support this API (this shouldn't be difficult to add to this PR)
Seems like some good answers/solutions here: https://github.com/WebDevSimplified/Face-Detection-JavaScript/issues/9#issuecomment-544165747
I have updated the PR to use the newer navigator.mediaDevices.getUserMedia
API, since it seems to be better supported than navigator.getUserMedia
(and navigator.getUserMedia
is also now deprecated). However, it seems that dart:html doesn't provide bindings for the new API, so I had to create them myself.
I have tested the updated code against both Chrome and Firefox on desktop and Firefox Preview and Chrome on Android and it continues to work fine. @treeder could you check again on iPhone?
Nope, different error now:
Which iOS version is the device running? Is this on Safari?
It's on Chrome, iOS 13.3, iPhone 11.
Great, thank you. Have you checked the updated version on Safari? (I think I read somewhere that Chrome on iOS can't access the camera, only Safari, but will search again later to see if it's still the case)
The link I found is this: https://stackoverflow.com/questions/51501642/chrome-and-firefox-are-not-able-to-access-iphone-camera EDIT: See this bug (still open) https://bugs.webkit.org/show_bug.cgi?id=208667
The issue remains open, so it seems like it's still impossible to access the camera from Chrome or Firefox on iOS, only Safari supports it. @treeder Could you check if it works on Safari?
Ya, looks like. But it does work on Safari so that's good enough for now. Could we return a good catchable error or something though so I can show a "use Safari" message?
Great! Thank you for testing it. Yes, I was planning on adding a more descriptive error if the browser doesn't support this API (while don't breaking the plugin API). I'll update this PR and will keep you updated.
@treeder I've updated the PR. I've updated it to first make it behave like the Android and iOS platforms (throwing a PlatformException
when the user cancels the scan and also when the user denies access to the camera), and I also added a new error code (BarcodeScanner.CameraAccessNotSupported
) which is thrown when the browser doesn't support the required mediaDevices
API.
@fercarcedo ya know what might be useful, a canScan()
method to check before trying. Kind of like the canLaunch(url)
method in the url_launcher
plugin.
Bit I suppose that might require a change to the entire API, not just web right?
Yes, I think this would require making changes to the plugin API, not just the web part.
What you suggest seems to me like a good idea, but since it needs to modify the plugin API I think @devtronic would need to approve it
is it possible using xzing-js rather than JsQRScanner ?! regards
@senatormas Yes, it could be possible to switch to the zxing-js library (in fact, it wouldn't require too many changes), but I wonder what are the advantages of doing so. Is this library more widely used? Is it better? Have you used it?
JsQRScanner is also based on ZXing (in fact, is generated automatically from the Java sources with a few modifications to make it work with HTML, and so one big advantage of it is that it's always updated with the latest version of ZXing, which doesn't seem to be the case with zxing-js: see this issue zxing-js/library#261).
If there any advantages in doing so, I'll hapily migrate to this library, but I'd like to make an informed decision as much as possible :)
@fercarcedo thanks a lot for your reply. the only thing that is really useful about this migration is supporting different types of barcodes specially 1d barcodes. thats the main point. I have checked different aspects of current project using different platforms. I preferred using pwa on ios so I created a selfsigned cert using openssl in order to be able using camera on pwa. my final result was success on ios failing on android chrome and fortunately success on android firefox and even success on windows chrome . there was no problem with android apk as it was expected. the only thing that is really useful is supporting 1d barcodes on JsQRScanner. controlling flash light or camera side is not as much as important the 1d support is.
I'll explore it then, thank you
I find @zxing-js , despite being very immature, a very usefull library. If you choose not to use it, please let me know why. Thanks.
Yes, I already started migrating the code to use zxing-js. I'll update this PR soon with it :)
any good news about migration?! can I do anything for you?
This PR introduces web support, using the JsQRScanner JavaScript library.
The interface is implemented in HTML and CSS and tries to resemble as much as possible the one found on the mobile versions.
As a limitation, this web version doesn't support enabling the flashlight. In order to check if a device has a flashlight and enable it, we need to use the
ImageCapture
class which, at the time of opening this PR, has support problems with Firefox and Safari. It also doesn't seem to offer the ability to turn off the flashlight once turned on (once you request the flashlight to be turned on this seems to stick to the current video track session and it's on until you start a whole new track).This PR is implemented as a separate plugin. This seems to be the way to do it, as this post details https://medium.com/flutter/how-to-write-a-flutter-web-plugin-5e26c689ea1 (given the fact that this plugin does not follow the federated style). If you want it implemented in any other way, let me know :)
Closes #190