Conversation
eb8aee6 to
bd8bc44
Compare
There was a problem hiding this comment.
There is some stubbing of BrowserCache in this file that I think we should remove now that the step is no longer referencing it.
There was a problem hiding this comment.
Can we add / update specs for this file.
|
Are we expecting to fix Android tablets with this work? |
We are kicking the can, because no one on the team has access to an Android tablet at the moment |
There was a problem hiding this comment.
This spec predates the addition of the useDefineProperty test helper, which can help deal with some of the boilerplate around defining and restoring properties this way.
import { useDefineProperty } from '@18f/identity-test-helpers';
describe('isIPad', () => {
const defineProperty = useDefineProperty();
context('with ipad (old user agent format)', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
value:
'Mozilla/5.0(iPad; U; CPU iPhone OS 3_2 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Version/4.0.4 Mobile/7B314 Safari/531.21.10',
});
});
it('returns true', () => {
expect(isIPad()).to.be.true();
});
});
});There was a problem hiding this comment.
Related to my prior comment, I don't see that we're restoring the original value after this test is run.
There was a problem hiding this comment.
I've gone ahead and updated the whole file to use the test helper (f24deca). LMK what you think
There was a problem hiding this comment.
After playing around with the test helper for a couple of hours this afternoon, it seems like it won't play nice with navigator and the need to set write permissions on the object (and then turn them off).
There was a problem hiding this comment.
I think there were two issues with the previous commit:
- You were manipulating some global properties outside test lifecycle (particularly this one)
- Redefining the properties required using the
configurableoption
I was able to get it to pass after those were fixed up.
import {
isLikelyMobile,
hasMediaAccess,
isCameraCapableMobile,
isIPad,
} from '@18f/identity-device';
import { useDefineProperty } from '@18f/identity-test-helpers';
describe('isIPad', () => {
const defineProperty = useDefineProperty();
context('iPad is in the user agent string (old format)', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0(iPad; U; CPU iPhone OS 3_2 like Mac OS X; en-us) AppleWebKit/531.21.10 (KHTML, like Gecko) Version/4.0.4 Mobile/7B314 Safari/531.21.10',
});
});
it('returns true', () => {
expect(isIPad()).to.be.true();
});
});
context('The user agent is Macintosh but with 0 maxTouchPoints', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.125 Safari/537.36',
});
});
it('returns false', () => {
expect(isIPad()).to.be.false();
});
});
context('The user agent is Macintosh but with 5 maxTouchPoints', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.125 Safari/537.36',
});
defineProperty(navigator, 'maxTouchPoints', { configurable: true, value: 5 });
});
it('returns true', () => {
expect(isIPad()).to.be.true();
});
});
context('Non-Apple userAgent, even with 5 maxTouchPoints', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (Linux; Android 13) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/105.0.5195.58 Mobile Safari/537.36',
});
defineProperty(navigator, 'maxTouchPoints', { configurable: true, value: 5 });
});
it('returns false', () => {
expect(isIPad()).to.be.false();
});
});
});
describe('isLikelyMobile', () => {
const defineProperty = useDefineProperty();
context('Is not mobile and has no touchpoints', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.125 Safari/537.36',
writable: true,
});
defineProperty(navigator, 'maxTouchPoints', { configurable: true, value: 0 });
});
it('returns false', () => {
expect(isLikelyMobile()).to.be.false();
});
});
context('There is an Apple user agent and 5 maxTouchPoints', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.125 Safari/537.36',
});
defineProperty(navigator, 'maxTouchPoints', { configurable: true, value: 5 });
});
it('returns true', () => {
expect(isLikelyMobile()).to.be.true();
});
});
context('There is an explicit iPhone user agent', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (iPhone; CPU iPhone OS 12_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148',
});
});
it('returns true', () => {
expect(isLikelyMobile()).to.be.true();
});
});
});
describe('hasMediaAccess', () => {
const defineProperty = useDefineProperty();
context('No media device API access', () => {
beforeEach(() => {
defineProperty(navigator, 'mediaDevices', { configurable: true, value: undefined });
});
it('returns false', () => {
expect(hasMediaAccess()).to.be.false();
});
});
it('Has media device API access', () => {
beforeEach(() => {
defineProperty(navigator, 'mediaDevices', { configurable: true, value: {} });
});
it('returns true', () => {
expect(hasMediaAccess()).to.be.true();
});
});
});
describe('isCameraCapableMobile', () => {
const defineProperty = useDefineProperty();
context('Is not mobile', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4147.125 Safari/537.36',
});
defineProperty(navigator, 'mediaDevices', { configurable: true, value: {} });
});
it('returns false', () => {
expect(isCameraCapableMobile()).to.be.false();
});
});
context('No media device API access', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (iPhone; CPU iPhone OS 12_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148',
});
defineProperty(navigator, 'mediaDevices', { configurable: true, value: undefined });
});
it('returns false', () => {
expect(isCameraCapableMobile()).to.be.false();
});
});
context('Is likely mobile and media device API access', () => {
beforeEach(() => {
defineProperty(navigator, 'userAgent', {
configurable: true,
value:
'Mozilla/5.0 (iPhone; CPU iPhone OS 12_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148',
});
defineProperty(navigator, 'mediaDevices', {});
});
it('returns true', () => {
expect(isCameraCapableMobile()).to.be.true();
});
});
});-- What As of iOS 12 / the newest iPadOS versions, sites are by default rendered with "full web" view mode, which causes the Safari on the devices to report the incorrect user agent (in this case, a Macintosh). This prevents normal methods for checking whether or not the device is an iPad. As a proxy, we check for an apple user agent and then see whether or not the device has `maxTouchPoints` of 5, which indicates an iPad. changelog: Improvements, Device Compatibility, updating iPad checks
-- What For some mobile devices -- iPad in particular -- we need explicit information from the frontend about the client browser's capabilities in order to properly determine if it is mobile and camera-ready. However, on the Rails backend, we've been performing server-side checking to determine whether or not a device is mobile. This check is more or less a user agent check, which will respond false for newer iPadOS devices. In this commit, we add to our device_mobile? method to also check for flow_session state that has been set from a hidden input on the frontend. The input and session state setting were already occurring -- this is not new code. What's new is checking for the presence of the skip_upload_step property in the session, which only get set on the frontend if the device is mobile.
changelog: Improvements, Acuant for Tablet, enabling Acuant capture for ipads
changelog: Improvements, Acuant Tablets, enabling acuant capture for ipad
changelog: Improvements, Tablet Compatibility, adding Acuant for iPad
-- What Previously, this test relied on the Browser module's reading of the User Agent, which was mocked to imitate an older mobile device. With recent changes we are instead using the flow_session's :skip_upload_step -- set indirectly by javascript on the frontend -- in the upload step's mobile_device? method. Here we simply stub that method in this particular test suite.
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
changelog: Improvements, Tablet Compatibility, enabling Acuant SDK for ipads
changelog: Improvements, Tablet Compatibility, adding iPad for Acuant SDK
changelog: Improvements, Tablet Compatibility, assing Acuant for iPad
changelog: Improvements, Tablet Compatibility, adding Acuant for iPad
changelog: Improvements, Tablet Compatibility, adding Acuant for iPad
changelog: Improvements, Tablet Compatibility, adding Acuant for iPad
changelog: Improvements, Testing, Updating mobile checks on frontend
changelog: Improvements, Refactoring, updating tests for frontend platform checks
04e2e0f to
f24deca
Compare
-- What Mocha evidently has some difficulty changing the writability of properties in before() functions when we are using the useDefineProperty helper. This fixes that by just declaring the properties we are concerned about as being writable at the top of the file. changelog: Improvements, Tablet Compatibility, updating frontend platform check tests
changelog: Improvements, Tablet Compatibility, updating platform check tests
What
This PR deals with LG-7277, for which it was discovered that the Acuant SDK was not being loaded on iPads or Android tablets.
Notes
It turns out that Apple has changed the User Agent header for Safari on iPadOS, such that the result indicates a Macintosh running Safari. The workaround that many people have been using is to detect, on the frontend, the
window.navigator.maxTouchPoints, which on a tablet like the iPad, will be 5 (and usually 0 on a laptop or desktop).After updating the checks we use to indicate mobile on the frontend, I found that the frontend sets a hidden input with a "skip_upload" value if the device is mobile, and that the Rails application will set the
flow_session[:skip_upload_step]based on this value.With that in mind, I've updated the controller's
mobile_device?method to also take this into consideration.