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

Does not work with cordova-plugin-wkwebview since 1.2.0 #127

Closed
andreialecu opened this issue Mar 28, 2016 · 22 comments
Closed

Does not work with cordova-plugin-wkwebview since 1.2.0 #127

andreialecu opened this issue Mar 28, 2016 · 22 comments

Comments

@andreialecu
Copy link
Contributor

I haven't updated in a while, and just tried doing so and ran into a problem.

There is some incompatibility in the way #47 was implemented which makes the web server used by the wkwebview plugin not be able to serve the files from "external storage".

The initial startup goes fine, but on the second start of the app it hangs on the splash screen and nothing loads. I connected Safari to the device and can see that the request for index.html returns a 404 from wkwebview's webserver.

The difference is that pre 1.2.0 the webview was being set to an url like http://localhost:12334/Application%20Support/..../www/index.html while the new version attempts to override the wwwFolder and simply set the location to /index.html.

Would it be feasible to revert to the old behavior? Is there any benefit from the new one?

@andreialecu
Copy link
Contributor Author

Also note that https://github.com/Telerik-Verified-Plugins/WKWebView/ does not support cordova-ios > 4, so I'm currently using 3.9.1.

For now, I reverted to 1.1.2 of CHCP which seems to not have this issue.

@andreialecu
Copy link
Contributor Author

Actually, I just tried 1.2.0 again and it worked. I kept installing newer versions until I found the one where this was broken. 1.2.1 works, but 1.2.2 doesn't.

@nikDemyankov
Copy link
Member

The difference is that pre 1.2.0 the webview was being set to an url like http://localhost:12334/Application%20Support/..../www/index.html while the new version attempts to override the wwwFolder and simply set the location to /index.html. Is there any benefit from the new one?

Yes, there are benefits from the new approach. You can look into documentation.

Actually, I just tried 1.2.0 again and it worked. I kept installing newer versions until I found the one where this was broken. 1.2.1 works, but 1.2.2 doesn't.

That's an interesting observation. Difference between these versions should not be big, just some bug fixes. Maybe I "fixed" something else as well...

@andreialecu
Copy link
Contributor Author

Actually I think there may have been some caching going on as much as I tried to avoid it. 1.2.0 on the physical device didn't work, but in the simulator it did. I think it was still using the old version somehow as much as I tried to avoid that.

I reverted to 1.1.2 for now.

@andreialecu
Copy link
Contributor Author

Yes, there are benefits from the new approach. You can look into documentation.

Not sure I see the part that refers to this in the documentation. The new separate folders per release feature is great, but it doesn't refer to the wwwFolder issue.

From what I can see while debugging the plugin attempts to set the wwwFolder directly to the Application Support release folder, and load /index.html from it and this is somehow incompatible with the cordova server plugin.

If it simply updated window.location to the Application Support/... folder like it did before 1.2.0 there wouldn't have been any problem.

It may be a bug in the cordova webserver plugin, I'm not denying that, but since the previous method worked, I'm just wondering why it was changed.

Edit:

But on every next launch/update - we will load an index page from the external storage.

Found just this, no explanation though as to why that is a better approach. :)

@nikDemyankov
Copy link
Member

Sorry for the late reply.

Found just this, no explanation though as to why that is a better approach. :)

Yeah, but if you continue reading:

For each release plugin creates a folder with this name on the external storage, and puts in it all your web files. It is a base url for your project. This approach helps to solve several problems:

  • Files caching issue. For example, on iOS css files are cached by the UIWebView, and even if we reload the index page - new styles were not applied. You had to kill the app in the task manager to flush it, or do some hacks to change the url of the css file.
  • Less chances that update will corrupt the existing content, since we are using totally different folders for each release.
  • But if it is corrupted - we can rollback to the previous version.

So, the main idea behind "new release -> new folder" approach is that it makes plugin more stable and less chances of breaking the app, since releases are now more independent.

I think soon chcp will support cordova-plugin-wkwebview-engine plugin. Then will try to make it work with https://github.com/Telerik-Verified-Plugins/WKWebView/.

@andreialecu
Copy link
Contributor Author

I think you are misunderstanding what I mean. The problem is not with the different release folders but with the way it sets the starting wwwfolder on subsequent app starts.

The older versions would redirect to a deep folder via javascript, the new versions try to set it from the backend and simply load /index.html

@nikDemyankov
Copy link
Member

The older versions would redirect to a deep folder via javascript

No, it was always reloading index page from the native side. Even in older versions.

The problem is not with the different release folders but with the way it sets the starting wwwfolder on subsequent app starts.

If I remember correctly, I had to switch to setting wwwFolder instead of startPage because of the issue with having # in the index.html path - ccdaffe.

Can double-check it later. But I suspect, that fix for telerik plugin will be similar to cordova-plugin-wkwebview-engine: set for the local server www folder as /some/path/Library/Application Support/<PACKAGE_NAME>/cordova-hot-code-push-plugin/.

@andreialecu
Copy link
Contributor Author

That is it then. With startPage it used to work.

@barocsi
Copy link

barocsi commented May 2, 2016

So for Telerik WKWebview users is this a better approach to revert to 1.1.2 or 1.2.0? Is there any breaking change in later releases?

@barocsi
Copy link

barocsi commented May 2, 2016

I have reverted to 1.1.2 despite using custom load options file seems to be xcompatible with 1.3.
@nikDemyankov are there any other issues I should be aware with 1.1.2 until you (if have some more energy) can bring in WKWebview Telerik version alive?
Thanks

@nikDemyankov
Copy link
Member

@barocsi You can look into changelog or in closed milestones to see, what has changed since v1.1.2. I can try to make a branch, that might fix this issue, but it will work only if index page doesn't contain # in url.

@nikDemyankov
Copy link
Member

@andreialecu If you want - you can try out current version with telerik wkwebview. To do that - use the following command to add plugin:

cordova plugin add https://github.com/nordnet/cordova-hot-code-push.git#telerik-wkwebview-fix

It should work, if your index page does not have # in it's url.

@andreialecu
Copy link
Contributor Author

The branch works. Thanks!

@nikDemyankov
Copy link
Member

Glad to hear that :)

@nikDemyankov nikDemyankov added this to the v1.4.0 milestone May 9, 2016
@nikDemyankov nikDemyankov removed this from the v1.4.0 milestone Jun 21, 2016
@barocsi
Copy link

barocsi commented Aug 6, 2016

is this merged in the new release?

@nikDemyankov
Copy link
Member

@barocsi Branch telerik-wkwebview-fix is not merged into the master, since there is still a problem with #, if it is used in the index page. But I can merge master into that branch, so you could use it, if needed. But be advised, that this is a fix for telerik version of wkwebview, not the https://github.com/apache/cordova-plugin-wkwebview-engine .

@nikDemyankov
Copy link
Member

Merged master into telerik-wkwebview-fix branch.

@kristfal
Copy link

@nikDemyankov Thanks for creating the telerik-wkwebview-fix, it works just as expected.

I'm wonder, could you merge the latest master into that branch again? In particular, getVersionInfo is missing, and it would be awesome to have that the branch without doing a personal fork of the branch.

@nikDemyankov
Copy link
Member

@kristfal sure, will do :)

@nikDemyankov
Copy link
Member

@kristfal merged and pushed the changes. Please, check it out. Should work, but I could have miss something...

@nordnet-deprecation-bot
Copy link
Contributor

👋 Hi! Thank you for your interest in this repo.

😢 We are not using nordnet/cordova-hot-code-push anymore, and we lack the manpower and the experience needed to maintain it. We are aware of the inconveniece that this may cause you. Feel free to use it as is, or create your own fork.

🔒 This will now be closed & locked.

ℹ️ Please see #371 for more information.

@nordnet nordnet locked and limited conversation to collaborators Sep 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants