browser icon indicating copy to clipboard operation
browser copied to clipboard

BrowserMultiFormatOneDReader no longer working

Open RobertAKARobin opened this issue 4 years ago • 10 comments

I was notified two days ago that none of my users were able to scan barcodes. This was weird to me because I hadn't changed the code at all. Then I noticed that a this library had issued a new release 2 days ago, and I was loading https://unpkg.com/@zxing/browser@latest. Simply switching to https://unpkg.com/@zxing/[email protected] fixes the problem.

It seems this is a known issue in zxing-js/library: https://github.com/zxing-js/library/issues/394.

Since it was in this recent batch of releases that /browser upgraded its dependency on /library from 0.18.2 to 0.18.3, I suspect the root cause is in /library's 0.18.3 release.

I also notice that with the new release the infamous NotFoundException is now present, which is a shame because that noticeably impacts the library's performance.

RobertAKARobin avatar Jan 29 '21 19:01 RobertAKARobin

Thanks for reaching out, I'll investigate on the issue when I find some time. Can you explain further what you tried to make the scanner to work? Did you try any specific barcode format hint or a specific reader to see if the issue was with that format? Have you tried to update the @zxing-js/library inside your project instead of the browser layer?

odahcam avatar Feb 01 '21 21:02 odahcam

Yes, I tried using the latest versions of /library and it didn't work, I suppose for the above reason.

I was using the BrowserMultiFormatOneDReader and trying various UPC codes from products lying around my desk.

I found that if I explicitly set DecodeHintType.POSSIBLE_FORMATS to [$someValue], where $someValue is literally any truthy value, then it works as expected, and as it did in v0.0.3 (although the annoying NotFoundException is still present).

RobertAKARobin avatar Feb 01 '21 22:02 RobertAKARobin

Maybe it makes the multi format reader to enable the 1D barcodes by interpreting the truth value as a number and matching it with some valid barcode format enum values. By default the CODE 128 should be enabled with all the other 1D readers only if no possible formats were enabled. I think we should investigate in here: https://github.com/zxing-js/library/blob/e7d06784f0b561040a18253556f9c4a50bd8cbd3/src/core/oned/MultiFormatOneDReader.ts#L40

odahcam avatar Feb 05 '21 23:02 odahcam

Can also confirm this with BrowserMultiFormatOneDReader/BrowserMultiFormatReader and decodeFromStream. v0.0.3: working v0.0.4 - v0.0.8: video, NotFoundException v0.0.9: no video, no exception

rubenpoppe avatar Apr 19 '21 09:04 rubenpoppe

Thanks for this info, I'll investigate this in the next days.

odahcam avatar Apr 20 '21 21:04 odahcam

I think I found the chunk of code responsible for this issue. This is from the unminified UMD script at https://unpkg.com/@zxing/browser@latest/umd/zxing-browser.js

try {
    // const result: Result = reader.decodeRow(rowNumber, row, startGuardPattern, hints);
    var result = reader.decodeRow(rowNumber, row, hints);
    // Special case: a 12-digit code encoded in UPC-A is identical to a "0"
    // followed by those 12 digits encoded as EAN-13. Each will recognize such a code,
    // UPC-A as a 12-digit string and EAN-13 as a 13-digit string starting with "0".
    // Individually these are correct and their readers will both read such a code
    // and correctly call it EAN-13, or UPC-A, respectively.
    //
    // In this case, if we've been looking for both types, we'd like to call it
    // a UPC-A code. But for efficiency we only run the EAN-13 decoder to also read
    // UPC-A. So we special case it here, and convert an EAN-13 result to a UPC-A
    // result if appropriate.
    //
    // But, don't return UPC-A if UPC-A was not a requested format!
    var ean13MayBeUPCA = result.getBarcodeFormat() === BarcodeFormat$1.EAN_13 &&
        result.getText().charAt(0) === '0';
    // @SuppressWarnings("unchecked")
    var possibleFormats = hints == null ? null : hints.get(DecodeHintType$1.POSSIBLE_FORMATS);
    var canReturnUPCA = possibleFormats == null || possibleFormats.includes(BarcodeFormat$1.UPC_A);
    if (ean13MayBeUPCA && canReturnUPCA) {
        var rawBytes = result.getRawBytes();
        // Transfer the metadata across
        var resultUPCA = new Result(result.getText().substring(1), rawBytes, rawBytes.length, result.getResultPoints(), BarcodeFormat$1.UPC_A);
        resultUPCA.putAllMetadata(result.getResultMetadata());
        return resultUPCA;
    }
    return result;
}
catch (err) {
    // continue;
}

As a workaround I have taken the following steps:

  • Search the above file for the comment // UPC-A is covered by EAN-13
  • Replace this comment with the this function call: readers.push(new UPCAReader());
  • Just above this, remove the word else from the line else if (possibleFormats.indexOf(BarcodeFormat$1.UPC_A) > -1)

Seems to be working as expected so far.

besworks avatar Apr 22 '21 22:04 besworks

@besworks I can indeed confirm that your changes fix the UPC-A decoding. I have created a fork with your changes over at pxlchx/library and plan to merge these into upstream. Do you know of any potential unintented side effects this might cause or tests which one could run to check if everything works fine (I've tested EAN-13 and UPC-A codes manually so far)?

pojntfx avatar Aug 30 '21 14:08 pojntfx

@pojntfx I haven't run into any side-effects as of yet but I also haven't rigorously tested this either. The client I was working with at the time hasn't reported any issues since I implemented this workaround. They are not using EAN-13, only UPC-A so I can't say whether or not there would be any issues if supporting both formats is required.

besworks avatar Aug 30 '21 16:08 besworks

@besworks Thanks for the info! From my tests it seems to be working as expected too with both formats enabled. Let's get this merged :)

pojntfx avatar Aug 31 '21 07:08 pojntfx

@besworks @pojntfx Thanks too, I was blocked with this issue; from my tests all my format works fine.

ghost avatar Oct 13 '21 08:10 ghost