Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RNMT-3220 WKWebView ::: InAppBrowser doesn't open url's with OutSystems Apps #5

Merged
merged 96 commits into from
Sep 10, 2019

Conversation

usernuno
Copy link

@usernuno usernuno commented Sep 6, 2019

Description

Pulls version 3.1.0 from upstream and additionally does some quite relevant stuff. Please review the PR one commit at the time and mind the context of each commit.

Context

Closes https://outsystemsrd.atlassian.net/browse/RNMT-3220

fix: merge issues caused by UIWebView and WKWebView implementation decouple:

We had a fix in CDVInAppBrowser.m that commented the code that allowed the UserAgent to be modified (RPD-2199).

Most of that class has been "exploded" into multiple other classes. This code in particular can be found in CDVUIInAppBrowser.m and CDVWKInAppBrowser.m.

This commit fixes a merge issue caused by this and passed the RPD-2199 fix to the new affected code.

fix: safe area inset in now being correctly used

Applied fix from apache#301 (comment).

This did not fix the background color of the top portion of the screen. Added additional "fixes" for that.

feat: use WKWebView engine when available

The plugin supports UIWebView and optionally WKWebView. The latter is only available if a WKWebView engine plugin such as the one we use in MABS 6 is available in the code.

Due to this we must still use the UIWebView version in MABS 3, 4 and 5 and choose between both in MABS 6. But considering Apple will soon stop accepting apps with UIWebView it does not make much sense to add an option for the user to choose which engine to use.

We are then taking advantage of an already calculated property that contains whether the WKWebView engine is available or not to decide which engine to use (which is WKWebView if available).

The original option is therefore rendered useless.

Type of changes

  • Feature (change which adds functionality)
    • BREAKING CHANGE (existing functionality will not work as expected due to new feature)
  • Fix (change which fixes an issue)
    • BREAKING CHANGE (existing functionality will not work as expected due to fix)
  • Performance (change which improves performance)
    • BREAKING CHANGE (existing functionality will not work as expected due to performance improvement)
  • Refactor (non-breaking change that is neither feature, fix nor performance)
  • Style (non-breaking change that only affects formatting and/or white-space)

Components affected

  • Android platform
  • iOS platform
  • JavaScript
  • OutSystems

Tests

Manual tests were performed:

  • Built apps in Android and iOS using MABS 3, 4, 5 and 6
  • Tested sample app built with MABS 5 and 6 in Android and iOS

Screenshots (if appropriate)

Checklist

  • Pull request title follows the format RNMT-XXXX <title>
  • Tests have been created
  • Code follows code style of this project
  • CHANGELOG.md file is correctly updated
  • Changes require an update to the documentation
    • Documentation has been updated accordingly

steinaragustli and others added 30 commits October 6, 2017 14:45
…nal parameter to swap position of navigationbuttons and close/done button
CB-14048: (android) allowedSchemes check empty string fix
…url-schemes

CB-14061: (android) comply with RFC 3986 for custom URL scheme handling
CB-12922 (ios): fix In-app browser does not cede control
(consistent with test script in package.json)
- skip some platform versions for now
- mark FUTURE TBD platform versions
When calling `.open()` with a target of `_system`, the InAppBrowser on iOS is both launching the URL in the system browser AND also broadcasting to open the URL within the app (calling handleOpenURL). The latter behavior is problematic in many circumstances (e.g. when you want to explicitly open a link in a browser which is a universal link handled by the app).

This commit attempts to address this by checking the return value from openURL -- if it does not open the URL successfully, then (and only then) the code falls back to broadcasting the event within the app to handleOpenURL.
On Android, if the app defines an intent-filter for a given URL, and
then tries to use inappbrowser to launch that URL via the _system
target, the default handler for that intent is the app itself.

That behavior can lead to circular loops, and ultimately is not what the
developer wants -- the link should be launched in a browser.

Because there is no easy way to find the "default" system browser on a
device, this solution will do two things:
1) Check if the app is one of the targets for this intent
2) If so, create a custom chooser with all other targets, excluding the
current app.

If the app is not a target, then the current (existing) behavior is
preserved.

The only real "downside" to this approach is that a default handler can no longer be set for these URLs within the app, and a chooser will be shown each time the user taps a link that opens in a new browser.

Fixes https://issues.apache.org/jira/browse/CB-10795
Ralph Gutkowski and others added 13 commits June 13, 2019 00:33
* Fix beforeload for Android <= 7
* Change Android version check conditional
)

* Update Travis CI configuration for new paramedic

* remove wrong ADDITIONAL_TESTS_DIR

* Update .travis.yml

* remove failing platform
# Conflicts:
#	.github/PULL_REQUEST_TEMPLATE.md
#	package.json
#	plugin.xml
#	src/ios/CDVInAppBrowser.m
#	tests/plugin.xml
…couple

We had a fix in CDVInAppBrowser.m that commented
the code that allowed the UserAgent to be modified
(RPD-2199).

Most of that class has been "exploded" into multiple
other classes. This code in particular can be found in
CDVUIInAppBrowser.m and CDVWKInAppBrowser.m.

This commit fixes a merge issue caused by this
and passed the RPD-2199 fix to the new affected
code.

References https://outsystemsrd.atlassian.net/browse/RNMT-3220
Applied fix from apache#301 (comment).

This did not fix the background color of the top
portion of the screen. Added additional "fixes"
for that.

References https://outsystemsrd.atlassian.net/browse/RNMT-3220
The plugin supports UIWebView and optionally
WKWebView. The latter is only available if a
WKWebView engine plugin such as the one
we use in MABS 6 is available in the code.

Due to this we must still use the UIWebView version
in MABS 3, 4 and 5 and choose between both in
MABS 6. But considering Apple will soon stop
accepting apps with UIWebView it does not make
much sense to add an option for the user to choose
which engine to use.

We are then taking advantage of an already calculated
property that contains whether the WKWebView engine
is available or not to decide which engine to use (which
is WKWebView if available).

The original option is therefore rendered useless.

References https://outsystemsrd.atlassian.net/browse/RNMT-3220
tests/package.json Show resolved Hide resolved
@usernuno usernuno merged commit 380c066 into outsystems Sep 10, 2019
@usernuno usernuno deleted the chore/RNMT-3220/merge-upstream branch September 10, 2019 10:05
GFOutsystems added a commit that referenced this pull request Dec 3, 2020
Re-added code that was mistakenly deleted when merged with fork version 4.0.0.

#5
GFOutsystems added a commit that referenced this pull request Dec 3, 2020
Re-added code that was mistakenly deleted when merged with fork version 4.0.0.

#5
GFOutsystems added a commit that referenced this pull request Dec 3, 2020
A fix introduced in the commit: #5

Due to changes on the API the fix had to be changed
GFOutsystems added a commit that referenced this pull request Dec 4, 2020
A fix introduced in the commit: #5

Due to changes on the API the fix had to be changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.