fix(ios): support for direct downloads of .kmp packages from within the in-app browser by jahorton · Pull Request #4568 · keymanapp/keyman

jahorton · 2021-03-02T08:19:20Z

~~Fixes~~ Mitigates #2197.

Seems that the root issue was that the UIWebView type is more restrictive in how links are followed. Since it's an in-app WebView, it doesn't want to perform downloads that good ol' Safari's perfectly happy to perform. So... we need to be more proactive and intercept .kmp links.

Then... it turns out that with all the nifty, type-aware KMP installation functionality from earlier in 14.0's development... I accidentally made non-type-aware KMP installation significantly trickier. Ah well.

jahorton · 2021-03-02T08:25:06Z

ios/keyman/Keyman/Keyman/KMWebBrowser/WebBrowserViewController.swift

+      ResourceDownloadManager.shared.downloadRawKMP(from: request.url!) { file, error in
+        // do something!
+        if let error = error {
+          let alert = ResourceFileManager.shared.buildSimpleAlert(title: "Error", message: error.localizedDescription)


Yes, that "Error" should be localized. Sadly, I noted that there are a couple of other matching, non-localized spots yet to handle... which are actually rather related:

keyman/ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift

Lines 267 to 282 in 0801d69

public func buildKMPError(_ error: KMPError) -> UIAlertController {

return buildSimpleAlert(title: "Error", message: error.localizedDescription)

}

public func buildSimpleAlert(title: String, message: String, completionHandler: (() -> Void)? = nil ) -> UIAlertController {

let alertController = UIAlertController(title: title, message: message,

preferredStyle: UIAlertController.Style.alert)

alertController.addAction(UIAlertAction(title: "OK",

style: UIAlertAction.Style.default,

handler: { _ in

completionHandler?()

}))

//UIApplication.shared.keyWindow?.rootViewController?.present(alertController, animated: true, completion: nil)

return alertController

}

(Lines 268 and 274, in particular.)

May as well address all three together, whether that be within this PR or a separate one.

jahorton · 2021-03-02T08:25:48Z

ios/keyman/Keyman/Keyman/KMWebBrowser/WebBrowserViewController.swift

+      let alertController = UIAlertController(title: "Cannot Open Page",
+                                              message: error.localizedDescription,
+                                              preferredStyle: UIAlertController.Style.alert)
+      alertController.addAction(UIAlertAction(title: "OK",


And probably these, too, which (despite how Git will show it) are unchanged lines.

jahorton · 2021-03-03T07:29:48Z

The noted i18n spots have been rounded up as part of #4577.

…ings fix(ios): adds i18n for some error alerts

mcdurdin · 2021-03-04T03:19:28Z

ios/keyman/Keyman/Keyman/KMWebBrowser/WebBrowserViewController.swift

  func webView(_ webView: UIWebView,
               shouldStartLoadWith request: URLRequest,
               navigationType: UIWebView.NavigationType) -> Bool {
+    if request.url?.lastPathComponent.hasSuffix(".kmp") ?? false {


This may not be 100% correct. We should be looking at the content-disposition header if it exists to determine the filename. We cannot assume that a URL will have a .kmp extension (e.g. it might be called something like file-downloader.php)

When using the keyman.com language search. the initial link is PHP, though that ends up resolving to a .kmp that this can then intercept.

I'm completely unfamiliar with such headers; have any good example ones you could point me at for this?

Like, for an actual .kmp.

I don't have one handy, but you could throw this onto a local website together with khmer_angkor.kmp (which you can even rename if you want, e.g. in the script below I renamed the copy I am reading from to example_khmer_angkor.dat just to illustrate). You could save this as download.php.

<?php $file = './example_khmer_angkor.dat'; header('Content-Type: application/octet-stream'); header('Content-Disposition: attachment; filename=khmer_angkor.kmp'); header('Content-Length: ' . filesize($file)); readfile($file);

I'll test with that shortly, but first I wanted to see what I can get from the keyboard search.

From the keyboard page for sil_ipa, the link seems to process in three stages:

Stage 1:

▿ https://keyman.com/keyboard/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 ▿ url : Optional<URL> ▿ some : https://keyman.com/keyboard/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - _url : https://keyman.com/keyboard/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - cachePolicy : 0 - timeoutInterval : 2147483647.0 ▿ mainDocumentURL : Optional<URL> ▿ some : https://keyman.com/keyboard/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - _url : https://keyman.com/keyboard/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - networkServiceType : __C.NSURLRequestNetworkServiceType - allowsCellularAccess : true ▿ httpMethod : Optional<String> - some : "GET" ▿ allHTTPHeaderFields : Optional<Dictionary<String, String>> ▿ some : 3 elements ▿ 0 : 2 elements - key : "Accept" - value : "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" ▿ 1 : 2 elements - key : "User-Agent" - value : "Mozilla/5.0 (iPhone; CPU iPhone OS 14_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148" ▿ 2 : 2 elements - key : "Referer" - value : "https://keyman.com/keyboards/sil_ipa" - httpBody : nil - httpBodyStream : nil - httpShouldHandleCookies : true - httpShouldUsePipelining : true

Stage 2:

▿ https://keyman.com/keyboards/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 ▿ url : Optional<URL> ▿ some : https://keyman.com/keyboards/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - _url : https://keyman.com/keyboards/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - cachePolicy : 0 - timeoutInterval : 2147483647.0 ▿ mainDocumentURL : Optional<URL> ▿ some : https://keyman.com/keyboards/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - _url : https://keyman.com/keyboards/download?id=sil_ipa&platform=ios&mode=standalone&cid=760108319.1607477546 - networkServiceType : __C.NSURLRequestNetworkServiceType - allowsCellularAccess : true ▿ httpMethod : Optional<String> - some : "GET" ▿ allHTTPHeaderFields : Optional<Dictionary<String, String>> ▿ some : 5 elements ▿ 0 : 2 elements - key : "Accept-Encoding" - value : "gzip, deflate, br" ▿ 1 : 2 elements - key : "Accept" - value : "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" ▿ 2 : 2 elements - key : "Referer" - value : "https://keyman.com/keyboards/sil_ipa" ▿ 3 : 2 elements - key : "User-Agent" - value : "Mozilla/5.0 (iPhone; CPU iPhone OS 14_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148" ▿ 4 : 2 elements - key : "Accept-Language" - value : "en-us" - httpBody : nil - httpBodyStream : nil - httpShouldHandleCookies : true - httpShouldUsePipelining : true

Stage 3:

▿ https://downloads.keyman.com/keyboards/sil_ipa/1.8.4/sil_ipa.kmp ▿ url : Optional<URL> ▿ some : https://downloads.keyman.com/keyboards/sil_ipa/1.8.4/sil_ipa.kmp - _url : https://downloads.keyman.com/keyboards/sil_ipa/1.8.4/sil_ipa.kmp - cachePolicy : 0 - timeoutInterval : 2147483647.0 ▿ mainDocumentURL : Optional<URL> ▿ some : https://downloads.keyman.com/keyboards/sil_ipa/1.8.4/sil_ipa.kmp - _url : https://downloads.keyman.com/keyboards/sil_ipa/1.8.4/sil_ipa.kmp - networkServiceType : __C.NSURLRequestNetworkServiceType - allowsCellularAccess : true ▿ httpMethod : Optional<String> - some : "GET" ▿ allHTTPHeaderFields : Optional<Dictionary<String, String>> ▿ some : 5 elements ▿ 0 : 2 elements - key : "Accept-Encoding" - value : "gzip, deflate, br" ▿ 1 : 2 elements - key : "Accept-Language" - value : "en-us" ▿ 2 : 2 elements - key : "Referer" - value : "https://keyman.com/keyboards/sil_ipa" ▿ 3 : 2 elements - key : "Accept" - value : "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" ▿ 4 : 2 elements - key : "User-Agent" - value : "Mozilla/5.0 (iPhone; CPU iPhone OS 14_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148" - httpBody : nil - httpBodyStream : nil - httpShouldHandleCookies : true - httpShouldUsePipelining : true

Setting that up under my local keymanweb.com development 'server' setup...

▿ http://keymanweb.com.local/test.php ▿ url : Optional<URL> ▿ some : http://keymanweb.com.local/test.php - _url : http://keymanweb.com.local/test.php - cachePolicy : 0 - timeoutInterval : 60.0 ▿ mainDocumentURL : Optional<URL> ▿ some : http://keymanweb.com.local/test.php - _url : http://keymanweb.com.local/test.php - networkServiceType : __C.NSURLRequestNetworkServiceType - allowsCellularAccess : true ▿ httpMethod : Optional<String> - some : "GET" ▿ allHTTPHeaderFields : Optional<Dictionary<String, String>> ▿ some : 2 elements ▿ 0 : 2 elements - key : "Accept" - value : "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8" ▿ 1 : 2 elements - key : "User-Agent" - value : "Mozilla/5.0 (iPhone; CPU iPhone OS 14_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148" - httpBody : nil - httpBodyStream : nil - httpShouldHandleCookies : true - httpShouldUsePipelining : true

There are no further stages; the next thing that occurs is a "Frame load interrupted" error. So that's all iOS will give me without very significant changes, unless I'm missing something.

Just to make clear the testing setup I used:

jahorton · 2021-03-04T08:03:46Z

So... while the solution implemented by this PR isn't optimal, as you've well pointed out... I think something only partly broken is still much better than something entirely broken. We can always implement a follow-up, altering our perspective on this PR from "fixes" to a "mitigates".

jahorton · 2021-03-04T08:25:43Z

Alternatively, if you feel comfortable addressing this, please be my guest; I feel like I'm in over my head with your request.

mcdurdin

So... while the solution implemented by this PR isn't optimal, as you've well pointed out... I think something only partly broken is still much better than something entirely broken. We can always implement a follow-up, altering our perspective on this PR from "fixes" to a "mitigates".

I did some more research online and it seems like this is an open problem with both WKWebView and UIWebView. All sorts of esoteric ideas on how to work around it but no simple onFileDownload approach such as one would hope for.

Given that, let's go ahead with your implementation as it stands.

jahorton added 3 commits March 2, 2021 09:30

fix(ios): workaround pt 1 - package request shunted to Safari

00c9571

fix(ios): app browser kmp downloads

55e39b8

fix(ios): fixes kbd display over welcome.htm

9847e55

jahorton added ios/ fix labels Mar 2, 2021

jahorton added this to the B14S7 milestone Mar 2, 2021

github-actions bot added ios/app/ ios/engine/ labels Mar 2, 2021

jahorton commented Mar 2, 2021

View reviewed changes

jahorton linked an issue Mar 2, 2021 that may be closed by this pull request

bug(ios): Attempting to install a package from within Keyman Browser gives error "Cannot open page" / "Frame load interrupted" #2197

Open

jahorton mentioned this pull request Mar 3, 2021

chore(ios): better visual feedback for keyboard search during poor internet connectivity #4573

Merged

fix(ios): adds i18n for some error alerts

a78d1d7

jahorton mentioned this pull request Mar 3, 2021

fix(ios): adds i18n for some error alerts #4577

Merged

jahorton and others added 3 commits March 4, 2021 08:56

change(ios): attempt at changing file encoding of localizable.strings

00515de

chore(ios): because xcode's encoding converter is broken

aba0a53

Merge pull request #4577 from keymanapp/fix/ios/unlocalized-error-str…

14eace1

…ings fix(ios): adds i18n for some error alerts

mcdurdin requested changes Mar 4, 2021

View reviewed changes

jahorton removed a link to an issue Mar 4, 2021

bug(ios): Attempting to install a package from within Keyman Browser gives error "Cannot open page" / "Frame load interrupted" #2197

Open

jahorton changed the title ~~fix(ios): fixes downloads of .kmp packages from within the in-app browser~~ fix(ios): support for direct downloads of .kmp packages from within the in-app browser Mar 4, 2021

mcdurdin approved these changes Mar 4, 2021

View reviewed changes

jahorton merged commit 361f568 into beta Mar 5, 2021

jahorton deleted the fix/ios/2197-app-browser-pkg-downloads branch March 5, 2021 02:15

jahorton mentioned this pull request Mar 5, 2021

bug(ios): Attempting to install a package from within Keyman Browser gives error "Cannot open page" / "Frame load interrupted" #2197

Open

	public func buildKMPError(_ error: KMPError) -> UIAlertController {
	return buildSimpleAlert(title: "Error", message: error.localizedDescription)
	}

	public func buildSimpleAlert(title: String, message: String, completionHandler: (() -> Void)? = nil ) -> UIAlertController {
	let alertController = UIAlertController(title: title, message: message,
	preferredStyle: UIAlertController.Style.alert)
	alertController.addAction(UIAlertAction(title: "OK",
	style: UIAlertAction.Style.default,
	handler: { _ in
	completionHandler?()
	}))

	//UIApplication.shared.keyWindow?.rootViewController?.present(alertController, animated: true, completion: nil)
	return alertController
	}

Uh oh!

Conversation

jahorton commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jahorton Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jahorton commented Mar 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jahorton commented Mar 4, 2021

Uh oh!

jahorton commented Mar 4, 2021

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jahorton commented Mar 2, 2021 •

edited

Loading

jahorton Mar 2, 2021 •

edited

Loading