You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2020/04/29 13:04:38 UTC
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz opened a new pull request #688: (ios): set "preferredContentMode" to mobile
NiklasMerz opened a new pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
ios
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
See #687
### Description
<!-- Describe your changes in detail -->
WIP to show what issue is talking about. See #687
### Testing
<!-- Please describe in detail how you tested your changes. -->
### 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](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] dpa99c commented on a change in pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
dpa99c commented on a change in pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#discussion_r435791089
##########
File path: README.md
##########
@@ -168,6 +168,16 @@ At the moment the only supported target in OSX is `_system`.
`_blank` and `_self` targets are not yet implemented and are ignored silently. Pull requests and patches to get these to work are greatly appreciated.
+### iOS Quirks
+
+Since iOS 13 iPads try to adapt their content mode / user agent for the optimal browsing experience. This may result in iPads having their user agent set to `Macintosh`. You can change this with the `PreferredContentMode` preference in "config.xml".
Review comment:
Can I suggest a slightly longer explanation?
Since iOS 13 forked into iPadOS 13, iPads try to adapt their content mode / user agent for the optimal browsing experience. This may result in iPads having their user agent set to `Macintosh` making it hard to detect them as mobile devices using user agent string sniffing. You can change this with the `PreferredContentMode` preference in "config.xml".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-686693778
Is this good to merge? Failing CI is a seperate issue being worked on.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on a change in pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on a change in pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#discussion_r485565650
##########
File path: src/ios/CDVWKInAppBrowser.m
##########
@@ -758,6 +758,15 @@ - (void)createViews
configuration.mediaPlaybackRequiresUserAction = _browserOptions.mediaplaybackrequiresuseraction;
}
+ if (@available(iOS 13.0, *)) {
+ NSString *contentMode = [self settingForKey:@"PreferredContentMode"];
+ if ([contentMode isEqual: @"mobile"]) {
Review comment:
```suggestion
if ([contentMode isEqual: @"mobile"]) {
```
Guess not
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639419585
I dont really know where `iPhone-XR` comes from. I will try different platforms..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] timbru31 commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
timbru31 commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639407712
Did the name change from `iPhone-XR` to
` { platform:iOS Simulator, id:4715F3CB-93A5-46AB-AAEA-D2E86B57C88B, OS:12.1, name:iPhone Xʀ }`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639431598
Looks like I cannot use the Xcode 11 image with this local configuration. paramedic needs to be fixed first.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): set "preferredContentMode" to mobile
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-637723376
I think preference totally makes sense and using this preference for the main webview, too. Changed this ways. Let me know what you think.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] timbru31 edited a comment on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
timbru31 edited a comment on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639407712
Talking about the Travis failure (https://travis-ci.org/github/apache/cordova-plugin-inappbrowser/jobs/694970579): Did the name change from `iPhone-XR` to
` { platform:iOS Simulator, id:4715F3CB-93A5-46AB-AAEA-D2E86B57C88B, OS:12.1, name:iPhone Xʀ }`? (Around line 1698)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] jcesarmobile commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
jcesarmobile commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639419836
I think travis changes should go outside of this PR, probably best to sent a separate PR and once the tests are good in that PR, merge it and rebase this one
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] erisu edited a comment on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
erisu edited a comment on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639423586
@timbru31 https://github.com/ios-control/ios-sim/blob/7624cc7af7e875a04f2f7dc3d5c78f2979eb9908/src/helpers.js#L88-L92
and `iPhone-XR` comes from here.
https://github.com/apache/cordova-paramedic/blob/dbdbdc379067b8b3450f1008a9922b91f254fa73/conf/pr/local/ios-10.0.config.json#L5
Which is used because of `- env: PLATFORM=local/ios-10.0`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): set "preferredContentMode" to mobile
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-638986429
@dpa99c Thanks for the review. Yes we need to document. Since we also would need that preference in the cordova-ios platform my plan was to do the documentation in cordova-docs where the other prefrences are. The PR for cordova-ios is to be done, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639043418
It makes sense to explain this a bit. I pushed an update for README.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639427970
> @NiklasMerz the change in CI configurations might fail because it does not exist in paramedic
>
> ` - env: PLATFORM=local/ios-13.0`
>
> https://github.com/apache/cordova-paramedic/tree/master/conf/pr/local
It did fail https://travis-ci.org/github/apache/cordova-plugin-inappbrowser/jobs/694985529. I reverted that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] jcesarmobile commented on pull request #688: (ios): set "preferredContentMode" to mobile
Posted by GitBox <gi...@apache.org>.
jcesarmobile commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639014650
I agree with dpa99c, the preference is also going to work on ios and should be documented on ionic-docs for ios, but since it also works in the plugin, should be mentioned in the plugin too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on a change in pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on a change in pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#discussion_r435802162
##########
File path: README.md
##########
@@ -168,6 +168,16 @@ At the moment the only supported target in OSX is `_system`.
`_blank` and `_self` targets are not yet implemented and are ignored silently. Pull requests and patches to get these to work are greatly appreciated.
+### iOS Quirks
+
+Since iOS 13 iPads try to adapt their content mode / user agent for the optimal browsing experience. This may result in iPads having their user agent set to `Macintosh`. You can change this with the `PreferredContentMode` preference in "config.xml".
Review comment:
```suggestion
Since the introduction of iPadOS 13, iPads try to adapt their content mode / user agent for the optimal browsing experience. This may result in iPads having their user agent set to Macintosh making it hard to detect them as mobile devices using user agent string sniffing. You can change this with the PreferredContentMode preference in "config.xml".
```
Sounds good to me. However I would try to avoid naming iPadOS an iOS fork.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] erisu commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
erisu commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639423586
@timbru31 https://github.com/ios-control/ios-sim/blob/7624cc7af7e875a04f2f7dc3d5c78f2979eb9908/src/helpers.js#L88-L92
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639077823
Tests are failing probably because CI uses Xcode 10 and this is new for iOS13 in Xcode 11.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] erisu edited a comment on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
erisu edited a comment on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639425529
@NiklasMerz the change in CI configurations might fail because it does not exist in paramedic
` - env: PLATFORM=local/ios-13.0`
https://github.com/apache/cordova-paramedic/tree/master/conf/pr/local
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] NiklasMerz merged pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
NiklasMerz merged pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] timbru31 commented on a change in pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#discussion_r485534723
##########
File path: src/ios/CDVWKInAppBrowser.m
##########
@@ -758,6 +758,15 @@ - (void)createViews
configuration.mediaPlaybackRequiresUserAction = _browserOptions.mediaplaybackrequiresuseraction;
}
+ if (@available(iOS 13.0, *)) {
+ NSString *contentMode = [self settingForKey:@"PreferredContentMode"];
+ if ([contentMode isEqual: @"mobile"]) {
Review comment:
Objective-C noob here, two spaces required?, some for two lines below
##########
File path: README.md
##########
@@ -168,6 +168,16 @@ At the moment the only supported target in OSX is `_system`.
`_blank` and `_self` targets are not yet implemented and are ignored silently. Pull requests and patches to get these to work are greatly appreciated.
+### iOS Quirks
+
+Since the introduction of iPadOS 13, iPads try to adapt their content mode / user agent for the optimal browsing experience. This may result in iPads having their user agent set to Macintosh making it hard to detect them as mobile devices using user agent string sniffing. You can change this with the PreferredContentMode preference in "config.xml".
Review comment:
```suggestion
Since the introduction of iPadOS 13, iPads try to adapt their content mode / user agent for the optimal browsing experience. This may result in iPads having their user agent set to Macintosh, making it hard to detect them as mobile devices using user agent string sniffing. You can change this with the `PreferredContentMode` preference in `config.xml`.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-inappbrowser] erisu commented on pull request #688: (ios): allow to set "preferredContentMode"
Posted by GitBox <gi...@apache.org>.
erisu commented on pull request #688:
URL: https://github.com/apache/cordova-plugin-inappbrowser/pull/688#issuecomment-639425529
@NiklasMerz the CI change, ` - env: PLATFORM=local/ios-13.0`, might fail because it does not exist in paramedic,
https://github.com/apache/cordova-paramedic/tree/master/conf/pr/local
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org