Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Added cordova-plugin-ionic-webview 2+ support for Android and iOS #454

Merged
merged 11 commits into from
Nov 15, 2018

Conversation

atftech-artyom
Copy link
Contributor

@atftech-artyom atftech-artyom commented Oct 9, 2018

  • Check whether cordova-plugin-ionic-webview 2+ is installed on Android by looking for com.ionicframework.cordova.webview.IonicWebViewEngine class. If so, it calls IonicWebViewEngine.setServerBasePath instead of navigating to file via this.mainWebView.loadUrlIntoView(url, false);

  • Check whether cordova-plugin-ionic-webview 2+ is installed on iOS by looking for setServerBasePath selector in webViewEngine. If so, splits NSUrl path components, removes last one, joins again and passes to webViewEngine as part of CDVInvokedUrlCommand.

@msftclas
Copy link

msftclas commented Oct 9, 2018

CLA assistant check
All CLA requirements met.

@atftech-artyom atftech-artyom changed the title Added cordova-plugin-ionic-webview 2+ support for Android Added cordova-plugin-ionic-webview 2+ support for Android and iOS Oct 9, 2018
@alexandergoncharov-zz
Copy link
Contributor

Hi @atftech-artyom,
Thanks for contributing!

It is good changes. I'll review it a bit later. It is in queue. Sorry for this delay.

@alexandergoncharov-zz
Copy link
Contributor

Hi @atftech-artyom ,
Sorry for this delay.

I just tested you changes and found that it doesn't fix issue.
For Ios I successfully got update but after reloading app I got white screen.
For Android I also successfully got update but after reloading I got binary version.
All tests was on fresh created Ionic app.

Could you please provide dev env which you used for testing your changes and do you have similar behaviour?

Thanks,
Alexander

@atftech-artyom
Copy link
Contributor Author

atftech-artyom commented Oct 16, 2018

Hi @alexandergoncharov

I did my tests on Ionic 4.0.0-beta.11 with CodePush native plugin and it worked like a charm.

Moreover, it is working right now both on Android and iOS. I have used ionic cordova plugin add https://github.com/atftech-artyom/cordova-plugin-code-push.git instead of ionic cordova plugin add cordova-plugin-code-push

Excerpt from config.xml

<engine name="android" spec="7.1.1" />
<engine name="ios" spec="4.5.5" />
<plugin name="cordova-plugin-ionic-webview" spec="^2.2.0" />
<plugin name="cordova-plugin-code-push" spec="git+https://github.com/atftech-artyom/cordova-plugin-code-push.git" />

Excerpt from package.json

{
  "name": "ionic-application",
  "version": "0.0.1",
  "author": "Ionic Framework",
  "homepage": "http://ionicframework.com/",
  "scripts": {
    "ng": "ng",
    "start": "ng serve",
    "build": "ng build",
    "test": "ng test",
    "lint": "ng lint",
    "e2e": "ng e2e"
  },
  "private": true,
  "dependencies": {
    "@angular/common": "~6.1.1",
    "@angular/core": "~6.1.1",
    "@angular/forms": "~6.1.1",
    "@angular/http": "~6.1.1",
    "@angular/platform-browser": "~6.1.1",
    "@angular/platform-browser-dynamic": "~6.1.1",
    "@angular/router": "~6.1.1",
    "@ionic-native/app-center-push": "^5.0.0-beta.21",
    "@ionic-native/camera": "5.0.0-beta.21",
    "@ionic-native/code-push": "^5.0.0-beta.21",
    "@ionic-native/core": "5.0.0-beta.21",
    "@ionic-native/file": "^5.0.0-beta.21",
    "@ionic-native/http": "^5.0.0-beta.21",
    "@ionic-native/image-picker": "5.0.0-beta.21",
    "@ionic-native/native-storage": "^5.0.0-beta.21",
    "@ionic-native/splash-screen": "5.0.0-beta.21",
    "@ionic-native/status-bar": "5.0.0-beta.21",
    "@ionic/angular": "^4.0.0-beta.12",
    "angular-progress-bar": "^1.0.8",
    "code-push": "^2.0.6",
    "cordova-android": "7.1.1",
    "cordova-ios": "4.5.5",
    "cordova-plugin-advanced-http": "^2.0.1",
    "cordova-plugin-appcenter-push": "^0.1.8",
    "cordova-plugin-appcenter-shared": "^0.1.8",
    "cordova-plugin-camera": "^4.0.3",
    "cordova-plugin-code-push": "git+https://github.com/atftech-artyom/cordova-plugin-code-push.git",
    "cordova-plugin-compat": "^1.2.0",
    "cordova-plugin-device": "^2.0.2",
    "cordova-plugin-dialogs": "^2.0.1",
    "cordova-plugin-file": "^6.0.1",
    "cordova-plugin-file-transfer": "^1.7.1",
    "cordova-plugin-ionic-keyboard": "^2.1.3",
    "cordova-plugin-ionic-webview": "^2.2.0",
    "cordova-plugin-nativestorage": "^2.3.2",
    "cordova-plugin-splashscreen": "^5.0.2",
    "cordova-plugin-statusbar": "^2.4.2",
    "cordova-plugin-telerik-imagepicker": "^2.2.2",
    "cordova-plugin-whitelist": "^1.3.3",
    "cordova-plugin-zip": "^3.1.0",
    "core-js": "^2.5.3",
    "ng2-validation": "^4.2.0",
    "rxjs": "6.2.2",
    "rxjs-compat": "^6.3.3",
    "zone.js": "^0.8.26"
  },
  "devDependencies": {
    "@angular/cli": "~6.1.1",
    "@angular/compiler": "~6.1.1",
    "@angular/compiler-cli": "~6.1.1",
    "@angular/language-service": "~6.1.1",
    "@angular-devkit/architect": "~0.7.2",
    "@angular-devkit/build-angular": "~0.7.2",
    "@angular-devkit/core": "~0.7.2",
    "@angular-devkit/schematics": "~0.7.2",
    "@ionic/ng-toolkit": "^1.0.0",
    "@ionic/schematics-angular": "^1.0.0",
    "@types/jasmine": "~2.8.6",
    "@types/jasminewd2": "~2.0.3",
    "@types/node": "~10.9.2",
    "codelyzer": "~4.4.2",
    "jasmine-core": "~2.99.1",
    "jasmine-spec-reporter": "~4.2.1",
    "karma": "~3.0.0",
    "karma-chrome-launcher": "~2.2.0",
    "karma-coverage-istanbul-reporter": "~2.0.0",
    "karma-jasmine": "~1.1.1",
    "karma-jasmine-html-reporter": "^0.2.2",
    "protractor": "~5.4.0",
    "ts-node": "~7.0.0",
    "tslint": "~5.11.0",
    "typescript": "~2.9.2"
  },
  "description": "An Ionic project",
  "cordova": {
    "plugins": {
      "cordova-plugin-whitelist": {},
      "cordova-plugin-statusbar": {},
      "cordova-plugin-device": {},
      "cordova-plugin-splashscreen": {},
      "cordova-plugin-ionic-keyboard": {},
      "cordova-plugin-camera": {},
      "cordova-plugin-telerik-imagepicker": {
        "PHOTO_LIBRARY_USAGE_DESCRIPTION": "documents"
      },
      "cordova-plugin-file": {},
      "cordova-plugin-file-transfer": {},
      "cordova-plugin-zip": {},
      "cordova-plugin-appcenter-push": {},
      "cordova-plugin-ionic-webview": {},
      "cordova-plugin-code-push": {},
      "cordova-plugin-nativestorage": {},
      "cordova-plugin-advanced-http": {}
    },
    "platforms": [
      "android",
      "ios"
    ]
  }
}

Example code is a mess, as it was for demonstration purposes only, so excuse me for that:

async checkCodePush() {
        await this.codePush.sync(
            {
                installMode: InstallMode.IMMEDIATE,
            },
            (downloadProgress) => {
                this.downloadProgress(downloadProgress);
            }).subscribe(
            async (data) => {

                if (data.valueOf() === SyncStatus.DOWNLOADING_PACKAGE.valueOf()) {
                    if (!this.loader) {
                        this.loader = await this.loadingCtrl.create({
                            spinner: 'crescent'
                        });
                        this.loader.present();
                    }
                }

                if (data.valueOf() === SyncStatus.ERROR.valueOf() ||
                    data.valueOf() === SyncStatus.UPDATE_INSTALLED ||
                    data.valueOf() === SyncStatus.UPDATE_IGNORED) {
                    if (this.loader) {
                        await this.loader.dismiss();
                        this.loader = null;
                    }
                }
            },
            (err) => {
                console.log('CODE PUSH ERROR: ' + err);
                if (this.loader) {
                    this.loader.dismiss();
                    this.loader = null;
                }

            }
        );
    }

    async downloadProgress(downloadProgress) {

        if (downloadProgress) {
            this.onDownloadProgress = true;
            const progress = ((downloadProgress.receivedBytes / downloadProgress.totalBytes) * 100);
            // Update "downloading" modal with current download %
            console.log('Downloading ' + downloadProgress.receivedBytes + ' of ' + downloadProgress.totalBytes);

            if (this.loader) {
                this.loader.message = progress.toFixed(0) + '%';
            }
        }
    }

@atftech-artyom
Copy link
Contributor Author

Hi @alexandergoncharov

I'm also attaching Ionic 3 application that is working with my forked repository.

@alexandergoncharov-zz
Copy link
Contributor

Hi @atftech-artyom ,
Sorry for delay.
Thanks for attaching test project.
I just tested it on IOS. Also, successfully got update but after reloading I got white screen. Could you please try to reload your app after updating and let me know your results?

1 similar comment
@alexandergoncharov-zz
Copy link
Contributor

Hi @atftech-artyom ,
Sorry for delay.
Thanks for attaching test project.
I just tested it on IOS. Also, successfully got update but after reloading I got white screen. Could you please try to reload your app after updating and let me know your results?

@alexhisen
Copy link

@atftech-artyom For me, the Android build fails to compile in your fork with:
platforms/android/src/com/microsoft/cordova/CodePush.java:542: error: local variable finalIonicWebViewEngineUrlPath is accessed from within inner class; needs to be declared final
setServerBasePath.invoke(ionicWebViewEngine, finalIonicWebViewEngineUrlPath);

@alexhisen
Copy link

@alexandergoncharov I also see the same issue where subsequent app launches after the first update fail to load completely. One way or another, currently code-push doesn't work right with either Ionic or plain WKWebView and it's been like this for over 6 months and really needs to be fixed to work at least with one of them or better both.

@whegar
Copy link

whegar commented Oct 30, 2018

Hi all,

hi @atftech-artyom

The android patch, still does not work for me, I think it needs some minimal adjustments.

I try to explain it. And please, sorry for my English

I think what's happening is that the Ionic Webview ionic is an HTTP server itself.
For fix it and use it, we just have to change the Ionic Plugin basepath, but not open the url with using the cordova interface the url with protocol file://.

By the way all our fetches fail because of this. For fix, just leave it, then Ionic web view will load the same HTTP url using another base path.

see an minimal change into navigateToURL function,

			if (url != null) {
			  CodePush.ShouldClearHistoryOnLoad = true;
			  if (this.hasIonicWebViewEngine) {
				try { // IONIC WEB VIEW
				code .. YOUR FIX
				code .. YOUR FIX
				// Not use loadUrlIntoView function, ionic webview plugin will open the same url http://localhost:8080 , but your fixed before changing the basepath
				 
			  } else {
				this.mainWebView.loadUrlIntoView(url, false); 
			  }
			}

if I do not have it like that, I have errors of this type, to open with file:// instead of using the plugin feature by opening a url http: //localhost:8080
Fetch API can not load file: ///data/user/0/com.whegar.sample/files/codepush/deploy/versions/e24998e3a0125183f148a4c5147a393b419c1fd1c7fddc016a64ae594acc155e/www/assets/config/endpoint.config.json.
URL scheme "file" is not supported.

You can see the behavior using chrome: // inspect observing the url of the document, using the Ionic webview this should always be http: // localhost: 8080, if the plugin changes it by protocol file, it changes everything, and some things will not work like in the original binary version

Other problem that I have seen debugging, is the use of the local variable hasIonicWebViewEngine, I think it should always be checked into navigateToURL or another function.

Because, the load order of the plugins affects, I always get false into in the navigateToURL function, so the check procedure does not work for me.

thanks for the fix I did not know where to start looking

@whegar
Copy link

whegar commented Nov 8, 2018

Hi all,

Exactly the same thing happens on iOS, the white screen, occurs by the load made when ionic webview is passed to it file: //.../..../..../..../index.html in the function redirectsStartPageToURL. I have a patch that makes it work, it looks like this...:

             (void)redirectStartPageToURL:(NSString*)packageLocation{
                NSURL* URL = [self getStartPageURLForLocalPackage:packageLocation];
                if (URL) {
                    if ([self isCDVWKWebViewEngineAmbient]) {
                        [self setServerBasePath:URL.path];
                        ((CDVViewController *)self.viewController).startPage = [self getConfigLaunchUrl]; // protocol http://   (surely http://localhost:8080)
                    } else {
                        ((CDVViewController *)self.viewController).startPage = [URL absoluteString]; // protocol file://
                    }
                }
            }

summary:

I added a function that checks if you are in a wkview ionic environment, (using part atftech-liran code fix)

another function called setServerBasePath, using part atftech-liran code fix for introspection.

How do you want me to proceed?
Fork to microsoft or fork to @atftech-artyom first to take it to microsoft.

Hi @atftech-artyom ,
Sorry for delay.
Thanks for attaching test project.
I just tested it on IOS. Also, successfully got update but after reloading I got white screen. Could you please try to reload your app after updating and let me know your results?

@alexandergoncharov-zz
Copy link
Contributor

Hi @whegar ,
Thanks for this investigation and contributing!

I think that it will be better if you make fork from @atftech-artyom 's branch for keeping his commits in repo. Unfortunately @atftech-artyom is silence in a long time unfortunately but he also did a big investigation and contributing.

I'll investigate your android drafts a bit later. Sorry for this delay. But if you found good approach for fixing this issue please feel free also create PR for that.

Thanks,
Alexander

@whegar
Copy link

whegar commented Nov 9, 2018

Hi all,
Hi @alexandergoncharov,

ok,

I have done what you tell me and I have made the patches that I think are needed, I have checked it on an application (no scaffolding application) and it works for me.
then I created a pull request on the @atftech-artyom fork.

my branch, forked from @atftech-artyom
pull request link #atftech-artyom#1

@whegar
Copy link

whegar commented Nov 9, 2018

@atftech-artyom For me, the Android build fails to compile in your fork with:
platforms/android/src/com/microsoft/cordova/CodePush.java:542: error: local variable finalIonicWebViewEngineUrlPath is accessed from within inner class; needs to be declared final
setServerBasePath.invoke(ionicWebViewEngine, finalIonicWebViewEngineUrlPath);

Hi, @alexhisen

this error happens because you are using java < 1.8,
I you change the variable declaration to

final String finalIonicWebViewEngineUrlPath = ionicWebViewEngineUrlPath;

the problem will dissapear,

Regards.

Uff, ok

While the patch is not available if you update the cordova-android version to> 7.0.0, I recommend the 7.1.0 is more stable for me.

But yes, it can be said that the entire plugin becomes incompatible for java <1.8.

I'm going to change it in @atftech-artyom fork

@alexandergoncharov-zz
Copy link
Contributor

Hi @whegar ,

Thanks for this! Good catch!
I tested your branch and successfully got update for Android and Ios.
Could you please open new PR with this changes and we continue conversation there?
I would like to make more tests and code review.

whegar and others added 2 commits November 9, 2018 14:33
Fixes to use the http server embedded in the Ionic WebView, and so we will use the http protocol instead of file: // protocol
@atftech-artyom
Copy link
Contributor Author

I am sorry for being non-responsive for some time, I was busy at work.

I have merged PR by @whegar

@whegar
Copy link

whegar commented Nov 9, 2018

Hi @alexandergoncharov

all code already in the original PR by @atftech-artyom.

@alexandergoncharov-zz
Copy link
Contributor

Hi @atftech-artyom and @whegar ,
Thanks for this!

I tested it on Ionic and clear Cordova. On Ionic all works perfect. I successfully got updates and LiveReload feature also works.
But on clear cordova I have issue on ios. After installing update I got error. I saw input for index.html.
image

Here is logs:
image

I don't see any changes for logic without CDVWKWebViewEngine but looks like it is broken.
Do you have any ideas why it can be happen?

@@ -48,7 +52,7 @@
private boolean didUpdate = false;
private boolean didStartApp = false;
private long lastPausedTimeMs = 0;

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please remove unnecessary spaces?

});

return;
} catch (ClassNotFoundException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please wrap this 3 catchs in one catch for more comfortable code reading?
It should be something like so: } catch (ClassNotFoundException | NoSuchMethodException | URISyntaxException e) {

try {
Class.forName("com.ionicframework.cordova.webview.IonicWebViewEngine");
return true;
} catch (ClassNotFoundException e) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please remove one tab for align } catch... with try?

((CDVViewController *)self.viewController).startPage = [URL absoluteString];
if ([self isCDVWKWebViewEngineAmbient]) {
[self setServerBasePath:URL.path];
// ((CDVViewController *)self.viewController).startPage = [self getConfigLaunchUrl]; it is not necessary
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please remove commented code?

@whegar
Copy link

whegar commented Nov 12, 2018

..........

I don't see any changes for logic without CDVWKWebViewEngine but looks like it is broken.
Do you have any ideas why it can be happen?

Hi @alexandergoncharov , @atftech-artyom

Could it be that you are using the android update over ios?, Can be be different in WWW content folder.

!!see the screenshot, "gap: 0...CoreAndroid...", it seems that is

If it is not that, tomorrow morning I look at it. But for now
I also agree that the logic has not been broken.

For the other refactoring issues, no problem, I will code the refactor.

And...

What do you think, if we change the function name for check the Ionic WebView?
Now in PR, in both platforms are called different

You choose -->

hasIonicWebViewEngine() or isCDVWKWebViewEngineAmbient() ?

maybe we leave the original name hasIonicWebViewEngine()

... and also, create a function in the java version called setServerPath as in the iOS version, to decouple functionality.

Regards,

@alexandergoncharov-zz
Copy link
Contributor

alexandergoncharov-zz commented Nov 13, 2018

Hi @atftech-artyom @whegar ,

Yeah, sorry. It was my fault. I created new clear project and successfully got update.
About naming: I think that it will be better for keeping hasIonicWebViewEngine name because we also have cordova-plugin-wkwebview-engine plugin. With this plugin Ios project also has CDVWKWebViewEngine file. hasIonicWebViewEngine should return true only for ionic 'CDVWKWebViewEngine`. Just tested - It works correct.

So, I'll be waiting your code changes about this naming and other review comments.

@whegar
Copy link

whegar commented Nov 13, 2018

Hi @alexandergoncharov

all code already again in the original PR by @atftech-artyom.

@atftech-artyom
Copy link
Contributor Author

Hi @alexandergoncharov @whegar

Code by @whegar has been merged to this PR.

Cheers.

@alexandergoncharov-zz
Copy link
Contributor

That's great!
Thanks for fast changes! You 2 are awesome!

Today and tomorrow I would like to test some other cases and ensure that all works as expected. If all works correct I'll merge it tomorrow and soon make new release with your changes.

Thanks,
Alexander

Removed unnecessary return statement, replaced `e.printStackTrace();` call with `Utilities.logException(e);`, fixed missing tab space.
Marked serverPath argument as final in order to allow usage within inner anonymous class.
@alexandergoncharov-zz
Copy link
Contributor

@atftech-artyom Thanks for quick update!

Copy link
Contributor

@alexandergoncharov-zz alexandergoncharov-zz left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants