Skip to content

LG-4392: Upgrade Acuant SDK to v11.5.0#5590

Merged
aduth merged 28 commits intomainfrom
aduth-lg-4392-acuant-11-5
Nov 10, 2021
Merged

LG-4392: Upgrade Acuant SDK to v11.5.0#5590
aduth merged 28 commits intomainfrom
aduth-lg-4392-acuant-11-5

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Nov 8, 2021

Why: As a user, I expect that login.gov keeps their vendor dependencies up-to-date, so that bug fixes that may improve my likelihood of proofing success have been incorporated.

The v11.5.0 release is a major release which improves detection and handling for an ongoing iOS 15 "GPU Highwater" issue, and which should resolve issues with iPhone Pro Max devices.

Full changelog: https://github.com/Acuant/JavascriptWebSDKV11/releases
Migration guide: https://github.com/Acuant/JavascriptWebSDKV11/blob/master/SimpleHTMLApp/docs/MigrationDetails.md

Note that we are upgrading from v11.4.3 to 11.5.0, so this also includes changes from v11.4.4, v11.4.5, v11.4.6, and v11.4.7.

Implementation Notes:

  • Acuant has split the SDK file into a few additional JavaScript files. Most notable of these for our reference is the new AcuantCamera.js
  • The initialization sequence has changed, which is explained in the migration guide above.
  • Internally, Acuant will set a cookie if a failure occurs for any reason. This can include the iOS 15 "sequence break" failure code, but also includes soft errors such as camera access declined. The proposed implementation here explicitly handles this in a few cases:
    • We unset the cookie if a failure occurs due to camera access declined. As noted in the inline code comment, this is to allow the user follow our messaging to change their browser settings and re-try.
    • We do not use Acuant's internal "manual capture" fallback, because this also includes the cropping and image metrics analysis. While we may want to allow this in the future, it is not included here currently because (a) we do not have a good UX experience for the pending crop and (b) we differentiate on capture source between "acuant" and "upload" (manual), but the Acuant fallback is somewhere in the middle.
  • Acuant now handles accessible state texts with an aria-live region, so we are able to remove our handling of this behavior. We still include accessible heading, instructions, and button click-to-capture, since these are not included in Acuant's implementation.
  • The FullScreen dialog is now shown with a black background, since in iOS the capture area no longer fills the full vertical space of the screen. This was a regression/change as of Acuant v11.4.4 which is apparently expected now per updated documentation "iOS will fill the width of the screen with the camera preview, typically with some unfilled space at the top and bottom".
  • When Acuant fails due to the iOS 15 "sequence break" failure, we will show a message to the user that "Camera failed to start, please try again". Unfortunately, we are unable to automatically launch the manual capture fallback, since the browser restricts this to occur only in immediate response to a user interaction (click, tap, etc).

@aduth aduth force-pushed the aduth-lg-4392-acuant-11-5 branch from d1d043a to 16f68c6 Compare November 9, 2021 15:28
@aduth aduth marked this pull request as ready for review November 9, 2021 15:39
@aduth
Copy link
Contributor Author

aduth commented Nov 9, 2021

CodeClimate is taking issue with "complex logic" hereabouts:

https://github.com/18F/identity-idp/blob/a677e2b456d74c79cd578fdba9d067514f905659/app/javascript/packages/document-capture/components/acuant-capture.jsx#L392-L394

It didn't seem to take the refactoring in a677e2b as an improvement. I'm not really convinced it's worth worrying about much?

@aduth aduth force-pushed the aduth-lg-4392-acuant-11-5 branch from 958181f to 2a2ac64 Compare November 9, 2021 17:35
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aduth
Copy link
Contributor Author

aduth commented Nov 9, 2021

A few small updates from bug bash testing:

  • We're no longer planning to show the dialog with a black background, removed in 0662932
  • To improve the experience in landscape mode, we now center the canvas in the dialog as of a11a718

aduth added 18 commits November 10, 2021 11:01
This reverts commit ffd83e51d3abe450fd997cae91fa20bdf4e86703.
**why**: to read from and set the cookie assigned by acuant v11.5.0 for graceful manual fallback capture
bypass acuant manual fallback, since (a) it introduces delays we're not yet accommodating for in UX and (b) it doesn't fit our current model of capture sources as "acuant" or "manual", where the acuant graceful fallback is somewhere in between the two (applies cropping and metrics, but not captured with guided frames)
**why**: was originally intended to prevent acuant from stopping manual capture when "end"-ing in response to failed capture / fullscreen dialog close. since we now bypass acuant's manual capture, it's not needed
**why**: simplify diff
**why**: avoid csp errors
This reverts commit 60676bc188a4631ab91bd4b503500f07a2868f6a.
**Why**: Previously relied on event bubbling when within the canvas fallback content.
@aduth aduth force-pushed the aduth-lg-4392-acuant-11-5 branch from 7cf8b6b to f5db1c0 Compare November 10, 2021 16:35
@aduth
Copy link
Contributor Author

aduth commented Nov 10, 2021

@anniehirshman-gsa Based on our discussion, I customized the error message specific to the iOS 15 crash (f5db1c0):

screenshot

(In this example, I was re-taking a photo, hence why the field already has a preview)

@aduth aduth mentioned this pull request Nov 10, 2021
@anniehirshman-gsa
Copy link
Contributor

@anniehirshman-gsa Based on our discussion, I customized the error message specific to the iOS 15 crash (f5db1c0):

screenshot

(In this example, I was re-taking a photo, hence why the field already has a preview)

LGTM, thanks for confirming with the screenshot!

@aduth aduth merged commit 8e771bd into main Nov 10, 2021
@aduth aduth deleted the aduth-lg-4392-acuant-11-5 branch November 10, 2021 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants