fix(web): allow to use file URLs on Android#16132
Conversation
User Test ResultsTest specification and instructions Test Artifacts |
| resolve(new Response(httpRequest.response, { status: httpRequest.status })) | ||
| }; | ||
| httpRequest.onerror = function () { | ||
| reject(new TypeError('Local request failed')) |
There was a problem hiding this comment.
Can we capture the error which is passed in here -- it's an Event object but there should be some error data somewhere there?
There was a problem hiding this comment.
Done. Although it doesn't seem that we get error data - e is of type ProgressEvent wich derives from Event, and neither has any error data.
| * limitation of the Fetch API fetch(). On Android we still have to | ||
| * explicitly allow file URLs. | ||
| */ | ||
| private fetch(uri: string): Promise<Response> { |
There was a problem hiding this comment.
I'd prefer to use this only in the Android and iOS apps (WebView) and not the Browser version
There was a problem hiding this comment.
Done. Although probably need to be refined when we implement the fix for iOS.
| getSettings().setAllowFileAccessFromFileURLs(true); | ||
| getSettings().setAllowUniversalAccessFromFileURLs(true); |
There was a problem hiding this comment.
Turns out we don't need setAllowUniversalAccessFromFileURLs. Added a comment for the other line.
|
Can confirm this fix does not currently work on iOS. |
This change fixes the blank keyboard problem on Android and iOS which was caused by using the Fetch API fetch() function which doesn't support file:// URLs. This change now uses the `XMLHttpRequest` class instead and sets the necessary flags on the Android side. For Keyman for Web nothing changes compared to earlier alpha versions, which means that file URLs shouldn't be used. This change also deletes `loadKeyboardHelper.ts` which wasn't used anywhere. Co-authored-by: Marc Durdin <marc@durdin.net> Fixes: #16096 Build-bot: release
e225015 to
0fd21c0
Compare
This change adds a `fetchKeyboardFunc` to `DOMKeyboardLoader` that allows mobile platforms to use a different method to fetch the keyboard data. WebView's `KeymanEngine` implements fetching the URL by using `XMLHttpRequest`.
eadcbe9 to
79fd7c8
Compare
Test Results
Note The Test Artifacts link for Android does not work, instead I navigated to the apk through the Test checks links. |
mcdurdin
left a comment
There was a problem hiding this comment.
This is definitely a straightforward patch; can we refactor it to be more object-oriented given our existing pattern with KeymanEngine: KeymanEngineBase?
Will we need to do similar work for fonts and lexical models?
| * Fetch API's fetch() which doesn't support file:// URLs. At least in | ||
| * Keyman for Android we still have to explicitly allow file URLs. | ||
| */ | ||
| private fetch(uri: string): Promise<Response> { |
There was a problem hiding this comment.
I would prefer to have this designed as a protected function in KeymanEngineBase which we override here, in KeymanEngineBase.ts it would be something like:
protected fetch(uri: string): Promise<Response> {
throw new Error('not implemented');
}
and in browser/src/KeymanEngine.ts:
protected fetch(uri: string): Promise<Response> {
return window.fetch(uri);
}
and then this would also be protected.
That moves the responsibility for defining what fetch does into the same place for both WebView and Browser components.
Tracking this in #16137 -- we were planning for Android test PRs to include the full tag in the file name:
Note that the 'Keyman for Android apk (old PRs)' link currently works in the Test Artifacts list. |
This change fixes the blank keyboard problem on Android which was caused by using the Fetch API fetch() function which doesn't support file:// URLs. This change now uses the
XMLHttpRequestclass instead and sets the necessary flags on the Android side.For Keyman for Web nothing changes compared to earlier alpha versions, which means that file URLs shouldn't be used.
This change also deletes
loadKeyboardHelper.tswhich wasn't used anywhere.Note that this PR only fixes things for Android, but not iOS where we have a similar problem, but which requires a slightly different solution.
Fixes: #16096
Build-bot: skip release:web,android
User Testing
TEST_ANDROID:
TEST_WEB: