In T208134#4701112, @Esanders wrote:

https://serratus.github.io/quaggaJS/ looks promising

In T208134#4701975, @Schnark wrote:

I myself am successfully using a JS compilation of zbar: https://github.com/Schnark/barcode-reader/blob/master/js/lib/zbar-worker.js, and IIRC the last time I tested it, it was much faster than quaggaJS, even though it also detects QR codes.

They main problem all these JS's have is that a live input via getUserMedia might not be able to properly focus (according to https://bugs.chromium.org/p/chromium/issues/detail?id=343894 it works in Chrome on Android, but probably still not on iOS), which for normal sized barcodes means that you just can't read them. Instead you have to take a photo with the camara app, select that photo instead of the live stream, repeat these steps if something went wrong, and delete all those photos manually once you are done. That's not very user friendly.

A very experimental (i.e. currently an experiment in Chrome only, might perhaps change sometime in the future) is https://wicg.github.io/shape-detection-api/#barcode-detection-api.

If you only want this feature in the Android app, then you should do the barcode scanning in Java, not in JavaScript.

Change 493494 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/Citoid@master] WIP DEMO barcode scanning

https://gerrit.wikimedia.org/r/493494

Testable here: https://visualeditor-prototype.wmflabs.org/w/index.php?title=Pride_and_Prejudice&mobileaction=toggle_view_mobile#/editor/0

Video throughput on my iPhoneX seems jittery, but this is an amazing feature.. SHIP IT ! ;)

Awesome! There's a slight delay with no loading animation in between when the barcode app closes and when the isbn gets inserted that is just a little too long for no feedback on my phone... is there any way to make that pending? And also, when you press back after denying camera access, you get an empty dialog. But yes ship it :D.

Change 493494 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/Citoid@master] WIP DEMO barcode scanning

https://gerrit.wikimedia.org/r/493494

In T216775#5071948, @Mvolz wrote:

Awesome! There's a slight delay with no loading animation in between when the barcode app closes and when the isbn gets inserted that is just a little too long for no feedback on my phone... is there any way to make that pending? And also, when you press back after denying camera access, you get an empty dialog. But yes ship it :D.

Can you make a screen recording? The dialog should be closing fairly instantly - if it isn't that could be the shutdown code for the library causes the main thread to hang.

In T216775#5084713, @Esanders wrote:

In T216775#5071948, @Mvolz wrote:

Awesome! There's a slight delay with no loading animation in between when the barcode app closes and when the isbn gets inserted that is just a little too long for no feedback on my phone... is there any way to make that pending? And also, when you press back after denying camera access, you get an empty dialog. But yes ship it :D.

Can you make a screen recording? The dialog should be closing fairly instantly - if it isn't that could be the shutdown code for the library causes the main thread to hang

Done: https://www.dropbox.com/s/wa9x5umn4hhx59v/mobizen_20190404_134118.mp4

I note the small camera icon in the top left of the system tray disappears at the same time as the barcode appears. I would suspect that the camera closing is causes the whole phone to freeze up for a second. Potentially we could emit the barcode detected event before closing the camera so at least the code is there when the phone freezes.

Changing .closed to .closing in the scanner widget appears to fix this, sending the detected code to the widget at the start of the teardown process, instead of the end.

@Mvolz I've pushed this change to the test server.

In T216775#5093447, @Esanders wrote:

@Mvolz I've pushed this change to the test server.

Great, that seems to have fixed it.

Now on patch demo (mobile only): https://patchdemo.wmflabs.org/wikis/cbe37578751b8297c6f29ebdd4406bb7/w/index.php/Test

I'm uneasy about this feature for security reasons. As far as I know, browsers will only ask for camera permissions once, and then the website is just allowed to use the camera whenever it wants.

I think this offers new nasty possibilities to someone who manages to take over an interface administrator account, or finds an XSS vulnerability anywhere in any of the code the sites run. Previously just about the worst thing they could do was fetching the IP address, but if our sites have camera permissions, they will also be able to take photos and record videos.

Maybe I am paranoid, but I wanted to point this out.

In T216775#6507158, @matmarex wrote:

I'm uneasy about this feature for security reasons. As far as I know, browsers will only ask for camera permissions once, and then the website is just allowed to use the camera whenever it wants.

There is precedent in us asking for sensor data from users with the nearby pages feature asking for location (https://en.m.wikipedia.org/wiki/Special:Nearby). AFAIK there have been no concerns raised about this.

I think this offers new nasty possibilities to someone who manages to take over an interface administrator account, or finds an XSS vulnerability anywhere in any of the code the sites run. Previously just about the worst thing they could do was fetching the IP address, but if our sites have camera permissions, they will also be able to take photos and record videos.

There are undoubtedly terrible things that can happen if an attacker gets hold of an interface admin account (at least for as long as it goes unnoticed). In addition to gathering IPs and granted sensor data (GPS/camera), the user could exploit trust in the domain to prompt for a password, or payment information from donors. Additional private information could be harvested from various API calls, or by scraping Special:Preferences. A attacker could ask a much larger number of users for sensor data, again exploiting the trust in our domain. I don't think it's correct to characterise this as a large change in the data at risk. There are other pieces of work being undertaken to address this (such as enforcing our CORS policy, which would it make it harder to export stolen data).

@matmarex and I will create a security review task for this

Hey @sbassett and @Jcross can we have a review of this ticket/attached patches, or do we need to formally complete this form.

@JTannerWMF - we've taken a look and would appreciate it if you would fill out the Security Readiness Review form. Thanks!

Per T269007#7067880 this has been security reviewed and just needs sign off from @marcella.

Test wiki created on Patch Demo by ESanders (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/1fe72fc2e1/w/

Test wiki created on Patch demo by ESanders (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/b9bf6906b0/wiki/Douglas Adams

Test wiki on Patch demo by ESanders (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/b9bf6906b0/w/

Test wiki created on Patch demo by ESanders (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/1346aed9a7/wiki/Douglas Adams

Test wiki on Patch demo by ESanders (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/1346aed9a7/w/

Test wiki on Patch demo by ESanders (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/1fe72fc2e1/w/

Test wiki created on Patch demo by ESanders (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/3b416c7370/wiki/Douglas Adams

Change 493494 merged by jenkins-bot:

[mediawiki/extensions/Citoid@master] Barcode scanning

https://gerrit.wikimedia.org/r/493494

This works as expected. However, I observed the following:

✅ Scanning old books with savageable ISBN barcode
❓ I wasn't able to add some books, even though it scanned correctly. Is this something we are in control of?
❓ I struggled a bit with going to the next line immediately after adding the citation

See https://photos.app.goo.gl/c2S1C2vXo1XZyxhRA for reference.

In T216775#8919583, @EAkinloose wrote:

This works as expected. However, I observed the following:

✅ Scanning old books with savageable ISBN barcode
❓ I wasn't able to add some books, even though it scanned correctly. Is this something we are in control of?

Not really - we use a third party to do the lookup and no database is complete.

❓ I struggled a bit with going to the next line immediately after adding the citation
See https://photos.app.goo.gl/c2S1C2vXo1XZyxhRA for reference.

Thanks - this looks like a general issue with the citation workflow, rather than the ISBN scanning part, so lets address it in a new task.

This task is good to be verified. Thanks for clarifying.

Test wiki on Patch demo by ESanders (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/3b416c7370/w/