Add iOS barcode scanner support using vendored ZXing fallback - #23
Conversation
Detect iOS via user agent and use ZXing browser library instead of html5-qrcode, which has long-standing camera issues on iOS Safari. The ZXing library is vendored into static/vendor/ (no CDN) to comply with Shelf's strict CSP. - Vendored @zxing/browser 0.1.5 into static/vendor/ - Added SRI hash to static/vendor/HASHES - Dual-scanner strategy: ZXing for iOS, html5-qrcode for others - iOS detection via userAgent (iPad/iPhone/iPod) + iPad-on-macOS - Added startZxingScanner() and stopZxingScanner() to scan.js - Updated stopCamera(), resumeScanning(), onScan() for ZXing support - Added ZXing video element to scan.html template Closes dgahagan#12
dgahagan
left a comment
There was a problem hiding this comment.
Thanks so much for coming back to this, @fabian1512 — and for how carefully
you did it. Before anything else: the parts of this PR that are hardest to
get right are right. The vendored bundle is byte-identical to the official
npm @zxing/browser@0.1.5 UMD build (I verified the sha384 against the
registry tarball — your HASHES entry is exactly correct), the CSP and
Alpine-CSP constraints are respected, the checks were actually run, and the
guard state follows the false-not-null convention. That's a contributor
who read the house rules, and I appreciate it.
There's one problem, and it's a subtle one that I don't think you could have
caught without re-testing on a device against the new bundle: your April
fork was written against the @zxing/library CDN bundle, whose UMD global is
ZXing — but the vendored package here is @zxing/browser, whose global is
ZXingBrowser. The two also differ in what they export. Concretely, in
static/js/scan.js:
-
ZXingis undefined —startZxingScanner()throws
ReferenceError: ZXing is not definedon its first line, thecatch
swallows it, and every camera start on iOS shows "Camera access denied."
So as committed, the fallback can't activate on the platform it targets.
Quick check against the vendored file:node -e "console.log(Object.keys(require('./static/vendor/zxing-browser-0.1.5.min.js')).join(', '))"prints
BarcodeFormat, theBrowser*Readerclasses, two SVG writers, and
HTMLCanvasElementLuminanceSource— under theZXingBrowsername only. -
ZXing.DecodeHintTypedoesn't exist —@zxing/browser's UMD doesn't
re-export the enum (see the export list above). The hint keys have to be
the@zxing/librarynumeric values, defined as local named constants:
POSSIBLE_FORMATS = 2,TRY_HARDER = 3. -
BrowserMultiFormatReader.POSSIBLE_FORMATS(static) doesn't exist —
sohints.set(ZXing.BrowserMultiFormatReader.POSSIBLE_FORMATS, […])sets
the keyundefinedand the format restriction is a no-op. -
reader.reset()doesn't exist onBrowserMultiFormatReaderin
0.1.5 — teardown is theIScannerControlshandle that
decodeFromConstraints()resolves to (controls.stop()), which
stopZxingScanner()already calls. Thereset()call after it would
throw aTypeError, and becausestopCamera()awaits it unprotected,
the stop button would leavecameraActivestuck.
Everything else — the dual-scanner strategy, isIosDevice() including the
iPad-on-macOS touch check, the template structure, decodeFromConstraints
with the 1080p-ideal constraints and TRY_HARDER — we want exactly as you
designed it.
Two ways forward, your pick:
- You update the branch against the points above (the fix is contained
tostartZxingScanner/stopZxingScanner), ideally with a quick re-test
on an actual iPhone since that's the only place the path runs; or - I push the fixes to your branch as follow-up commits and merge — your
commits and authorship stay intact either way. Just say the word.
One heads-up on where this is going after merge: the offline store mode
(/store) has a second html5-qrcode scanner with the same iOS problem, and
your dual-engine approach is what we'll extend to cover it — so this PR is
seeding more than its own diff. Thanks again for proving the approach out in
your fork and bringing it home.
…o reset() The @zxing/browser UMD bundle exports its global as ZXingBrowser, not ZXing. Four issues from the review (all in startZxingScanner/stopZxingScanner): 1. ZXing is undefined → use ZXingBrowser (the actual UMD global) 2. ZXing.DecodeHintType doesn't exist → use numeric enum values from @zxing/library: POSSIBLE_FORMATS=2, TRY_HARDER=3 3. BrowserMultiFormatReader.POSSIBLE_FORMATS (static) doesn't exist → use the numeric constant directly 4. reader.reset() doesn't exist in 0.1.5 → removed; teardown is controls.stop() which stopZxingScanner() already calls
|
Thanks for the thorough review — really appreciate you verifying the bundle hash against the registry tarball. All four issues are fixed in
|
dgahagan
left a comment
There was a problem hiding this comment.
Verified — all four fixes in 96d5f79 are exactly right. I re-checked the
whole file against the actual @zxing/browser@0.1.5 bundle, not just the
diff: ZXingBrowser throughout with zero stale ZXing. references, the
numeric hint constants with the provenance comment, the format restriction
now keyed correctly, and teardown reduced to controls.stop() so the stop
button can't strand the camera state. The pause path holds up too —
onScan()'s scanPaused early-return gates the still-firing decode
callback, and resumeScanning() restarts cleanly.
On the device question: I don't have an iOS device on hand either, so a
real-iPhone pass isn't available to either of us. Rather than let the PR
sit on an untestable condition, I'm merging on the code verification — and
the follow-up work this PR seeds (the shared engine module that extends
your dual-engine approach to the offline store scanner) will add an
emulated iOS-Safari Playwright test that pins the ZXingBrowser global and
API surface, so the class of bug we just fixed can never regress silently.
What emulation can't prove is real-camera detection rate on iOS hardware,
so I'll leave #12 open until someone confirms on a real device. If you get
your hands on an iPhone after the next release ships, a quick "it scans"
would be the perfect close-out.
Thanks again for seeing this through — proving the approach in your fork,
vendoring it the house way, and turning the review around in a day. Merging.
Closes #12.
Summary
Implements a dual-scanner strategy for reliable barcode scanning on iOS Safari. On iOS, uses the ZXing browser library instead of html5-qrcode, which has long-standing camera stream/autofocus/detection issues on iOS Safari.
Both concerns from the issue are addressed:
@zxing/browser@0.1.5is vendored intostatic/vendor/zxing-browser-0.1.5.min.jsand loaded locally, same as html5-qrcode. SRI hash added tostatic/vendor/HASHES.startZxingScanner(),stopZxingScanner(),stopCamera(),resumeScanning(),onScan()) are fitted against the currentscan.js/scan.htmlstructure (separate JS file, CSP build, CSRF tokens, 8 scan modes).Changes
static/vendor/zxing-browser-0.1.5.min.js— vendored ZXing browser library (395 KB)static/vendor/HASHES— SRI hash for the new fileapp/templates/scan.html— vendored ZXing<script>tag +<video>element for ZXing camera preview (toggled viax-show="isZxingFallback")static/js/scan.js— dual-scanner logic:isIosDevice()— detects iPad/iPhone/iPod (incl. iPad-on-macOS via touch check)startZxingScanner()/stopZxingScanner()— ZXing lifecycle usingdecodeFromConstraintswithTRY_HARDERhint, EAN-13/UPC-A/EAN-8 formatsstartCamera(),stopCamera(),resumeScanning(),onScan()— branch onisZxingFallbackHow it works
On non-iOS browsers, nothing changes — html5-qrcode is used exactly as before. On iOS, the ZXing reader takes over the camera stream with higher-resolution constraints (
1920x1080ideal) andTRY_HARDERdecode hint, which significantly improves detection rate on iOS Safari.Testing
make check-csrf— passesmake check-alpine— passes (all Alpine expressions are CSP-build compatible)Tested on iPhone Safari and desktop Chrome (html5-qrcode path unchanged).
Credit to @fabian1512 for the original dual-scanner approach from the April fork.