fix(ios): re-implement user agent overwrite

apache / cordova-plugin-file-transfer

Apache Cordova File Transfer Plugin

https://cordova.apache.org/

Apache License 2.0

595 stars 888 forks source link

fix(ios): re-implement user agent overwrite #268

Closed timbru31 closed 3 years ago

timbru31 commented 3 years ago

This closes #263 This closes #258

Platforms affected

iOS

Motivation and Context

This solves the issue in #258 and makes the plugin compatible with cordova-ios@6.
It also re-implements the user agent overwrite.

Description

Sadly, there is no "nice" way to obtain the WKWebView user agent other than evaluating JavaScript.

Testing

My local fork compiles and has the correct user agent attached when making the network requests.

Checklist

[ ] I've run the tests to see all new and existing tests pass
[ ] I added automated test coverage as appropriate for this change
[x] Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
[x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
[ ] I've updated the documentation if necessary

sithwarrior commented 3 years ago

Great stuff, any chance of getting this pushed to npm, so we can start using it?

timbru31 commented 3 years ago

We don’t give ETAs on new releases, but as you can see we have resumed development of this plugin. There is a lot of testing and cleanup to do though.

Sent from my iPhone

On 27. Aug 2020, at 9:27 AM, Martin Bech notifications@github.com wrote:

Great stuff, any chance of getting this pushed to npm, so we can start using it?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

jpike88 commented 3 years ago

+1 to a fast release, as this is blocking builds.

christiaan commented 3 years ago

@timbru31 this appears to break adding all the configured headers for me.

My assumption is that this call does not block untill the completionHandler is executed.

[self.webViewEngine evaluateJavaScript:@"navigator.userAgent" completionHandler:^(NSString* userAgent, NSError* error) {

I looked at https://developer.apple.com/documentation/webkit/wkwebview/1415017-evaluatejavascript but it does not indicate tell if it blocks till the completionHandler has run.

EDIT: I've added some NSLog statements and indeed it does not block when calling evaluateJavascript.

timbru31 commented 3 years ago

Thanks for investigating, I'll take a look.

christiaan commented 3 years ago

That is a quick reply :)

I was considering moving the userAgent determination to the plugin is initialization. This would also reduce the amount of evaluateJavascript calls to just one instead of for each request.

timbru31 commented 3 years ago

Are you willing to prepare a PR for that? 👍

christiaan commented 3 years ago

Seems that is also not possible. Since pluginInitialize is synchronous the same problem can occur.

Since the headers can be set from javascript, including the User-Agent header. What about adding a default in javascript instead and removing the User-Agent from all the native code?

christiaan commented 3 years ago

I've looked at the Android and Windows implementation, both do not add the User-Agent header in the plugin code.

Am I missing something or can it just be removed?

christiaan commented 3 years ago

I've created https://github.com/apache/cordova-plugin-file-transfer/pull/284 for this issue.