Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Platforms affected
Browser
Motivation and Context
Fixes a failing test:
Description
The fix isn't truly a fix, but I don't think this issue is fixable. Previously this worked when the browser had no screen orientation API I believe, but modern versions of browser does. Just because the API exists however doesn't mean it's usable. Chrome on desktop will have the API exists but attempting to use it will result with a DOMException stating that the device isn't supported for locking.
Additionally, testing this specific case is also problematic because the
lock
promise still gets resolved without rejection. You cannot catch or otherwise capture the DOMException. The DOMException is only ever printed to the JS console if you attempt to run the code without anything catching the promise (and the promise is then resolved later).It's not something I really like, but I don't think we have a better option, and I think this is better than having a false negative test.
The old unit test still exists, but is hidden behind a
isLockable
flag. It's generally expected that mobile devices will have a workinglock
API, should the browser cordova app ever be loaded on a mobile browser, thereforeisLockable
is set to true if it is believed the test is running on a mobile device (based on some userAgent keywords). This is however untested because I don't think we have a way to test browser platform on a mobile device.Testing
Ran cordova-paramedic manually.
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)