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.

Sign up for GitHub

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

Jump to bottom

feat(Android): Implement ionic-file and ionic-content urls #242

Merged
merged 2 commits into from
Dec 17, 2018
Merged

feat(Android): Implement ionic-file and ionic-content urls #242

merged 2 commits into from
Dec 17, 2018

Conversation

jcesarmobile
Copy link
Member

Implement ionic-file and ionic-content for Android

closes #204, closes #183

mlynch
mlynch suggested changes Dec 17, 2018
View reviewed changes
src/android/com/ionicframework/cordova/webview/AndroidProtocolHandler.java Outdated
public InputStream openContentUrl(Uri uri) throws IOException {
InputStream stream = null;
try {
stream = context.getContentResolver().openInputStream(Uri.parse(uri.toString().replace("ionic-content:///", "content://")));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure ionic-content is the right sheme here? Technically this is just a generic webview plugin and isn't ionic related. Would be the same question for Capacitor. How about app-content://?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to app-content and app-file, but as it's "Ionic WebView" I think it makes sense to use ionic as prefix on the schemes.
This schemes are only understood by the WebView, and we provide the logic to convert the file/content urls to something the WebView understands.

Specially in Capacitor I think it makes sense to have them like capacitor-*

src/android/com/ionicframework/cordova/webview/WebViewLocalServer.java Outdated
private final static String httpScheme = "http";
private final static String httpsScheme = "https";
private final static String ionicFileScheme = "ionic-file";
private final static String ionicContentScheme = "ionic-content";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above re: ionic-content and ionic-file

src/www/util.js Outdated
if (url.length === 0 || url[0] !== '/') { // ensure the new URL starts with /
url = '/' + url;
if (url.startsWith('content://')) {
return url.replace('content://', 'ionic-content:///');;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Watch for hard coded strings here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed on last commit

@jcesarmobile jcesarmobile merged commit 8ef0c30 into ionic-team:master Dec 17, 2018
@jcesarmobile jcesarmobile deleted the android-schemes branch December 17, 2018 19:12
@jcesarmobile jcesarmobile mentioned this pull request Dec 18, 2018
Closed
@Bramzor
Copy link

Bramzor commented Dec 22, 2018

Just tried this pull request but was not able to load android content. Also seems that the version for ionic does not work. Are there any other prerequisites apart from
cordova plugin rm cordova-plugin-ionic-webview --force
cordova plugin add cordova-plugin-ionic-webview@latest
To make sure cordova-plugin-ionic-webview 2.3.1 is installed. And use the function in util.js? I also noticed that in the latest git, ionic-content was replaced by a VARIABLE of which I cannot find the definition in this repo. So I tried replacing content:// with ionic-content:/// but that didnt work.

@jcesarmobile
Copy link
Member Author

This is unreleased, will be available on next release (3.0.0)
If you want to test this you can install from github using the repository url.
Also if you use angular you have to tell it to trust this url schemes, you can do it in different ways, one of them is using SafeReourceUrl and DomSanitizer

@Bramzor Bramzor mentioned this pull request Dec 24, 2018
Closed
@Bramzor
Copy link

Bramzor commented Dec 24, 2018

Currently for iOS I use 'http://localhost:8080/_file_' to get to local iOS content. Is there a same way to reach the android content:// which I can use before this PR is released?

@jcesarmobile
Copy link
Member Author

jcesarmobile commented Dec 24, 2018 •
edited
Loading

Use window.Ionic.WebView.convertFileSrc on the urls, it should always return a valid url depending on the webview plugin version.

But I think content urls were not supported on 2.3.1, just file ones

@Bramzor
Copy link

Bramzor commented Dec 24, 2018

Use window.Ionic.WebView.convertFileSrc on the urls, it should always return a valid url depending on the webview plugin version

For iOS this didnt work as the plugin (cordova-plugin-contacts) does not return a file:// on iOS but an url like /var/mobile/Containers/Data/Application/... (so you might want to add this to convertFileSrc ;-) .So I was able to get this file by appending the url like I mentioned. Had a lot of issues trying to use that convertFileSrc function so in the end I gave up trying that approach. Dont remember if I pushed it all the way to notice that it just didnt work or that I just gave up.

@jcesarmobile
Copy link
Member Author

I’ll report an issue to fix that before 3.0.0 release

Ionitron added a commit that referenced this pull request Jan 3, 2019
@Ionitron
chore(release): 3.0.0 [skip ci]
45ad9ed 
# [3.0.0](v2.3.1...v3.0.0) (2019-01-03)

### Bug Fixes

* **iOS:** Remove unused code ([#247](#247)) ([bceb17a](bceb17a))

### Features

* Allows configuration of Mixed Content Mode ([#240](#240)) ([486d412](486d412)), closes [#231](#231)
* **Android:** Implement ionic-file and ionic-content urls ([#242](#242)) ([8ef0c30](8ef0c30)), closes [#204](#204) [#183](#183)
* **iOS:** Remove GCDWebServer ([#244](#244)) ([0dee0cf](0dee0cf))
* **WebViewLocalServer.java:** return 404 error code when a local file is not found ([#217](#217)) ([f7a551e](f7a551e)), closes [#216](#216)

### BREAKING CHANGES

* **iOS:** Sets deployment-target to 11, so will only work on iOS 11+

* Address changes
* changes the default from 1 (never) to 0 (always)
* **WebViewLocalServer.java:** Until now, the Android part of the plugin was returning a 200 http code even though
the requested file didn't exist. This behavior was inconsistent with the historical behavior of the
iOS webView. This change makes them both work in the same manner but introduces a breaking change
for the current Android users that are expecting a 200 http code no matter what and are testing the
not found error just by checking if the body is null.
@Ionitron
Copy link
Collaborator

Ionitron commented Jan 3, 2019

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@erobertson42 erobertson42 mentioned this pull request Jul 3, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlynch mlynch mlynch approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
4 participants
@jcesarmobile @Bramzor @Ionitron @mlynch