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

ionic webview v4 changes things again... #491

Closed
1 of 3 tasks
captaincaius opened this issue Aug 6, 2019 · 4 comments
Closed
1 of 3 tasks

ionic webview v4 changes things again... #491

captaincaius opened this issue Aug 6, 2019 · 4 comments
Labels
chore feature good first contribution PR welcome released

Comments

@captaincaius
Copy link
Contributor

I'm submitting a...

  • Bug report
  • Feature request
  • Documentation issue or request

Current behavior

ionic webview 4 changes quite a few things about the origin:

  1. no port 8081 anymore - on android it's http://localhost
  2. on ios, the origin's scheme is ionic:// by default, but due to integrating with some google services, it's a good idea to override it to httpsionic:// instead (you can see some issues in their queue for details)

Expected behavior

Necessary: update the index.html cordova monkeypatch accordingly - I'm now using this:

if(/^localhost/.test(location.host) & !/^localhost:/.test(location.host)) {
  // ...
}

Optional: iosScheme to httpsionic and update whitelist stuff accordingly

Minimal reproduction of the problem with instructions

Environment

Others:
@sinedied
Copy link
Member

sinedied commented Aug 7, 2019

Is it broken with the current version? Or do we need to upgrade + fix?

@captaincaius
Copy link
Contributor Author

well right now the regex in index.html doesn't match the actual scheme/host:
https://github.com/ngx-rocket/generator-ngx-rocket/blob/master/generators/app/templates/src/_index.html#L59
because the configuration we have has it set to the defaults, which is ionic://localhost for ios and http://localhost for android.

Whether it's "broken" or not, I'm not sure, because I haven't actually reproduced the bug that necessitates this fix since a long while ago. But I can't imagine it walked away on its own.

So the two options are:

  1. Fix that line in index.html to match our current configuration
  2. First change the configuration slightly to make ios httpsionic://localhost and change index.html to match THAT (using the pattern I posted above).

Although #2 is a little more heavy-handed, it's a little safer for integrations (see ionic-team/cordova-plugin-ionic-webview#283 for example)

@captaincaius
Copy link
Contributor Author

Okay looks like I was wrong :D ... I didn't see the updates to config.xml.

It's currently NOT broken - index.html matches config.xml ... so with that in mind, changing the configuration for easier integration with other services is all that's left, which isn't something critical since not all users will need to do that.

sinedied added a commit that referenced this issue Mar 26, 2020
@sinedied
fix: webview config for ios/android (close #491)
34d37a1
sinedied pushed a commit that referenced this issue Mar 27, 2020
@semantic-release-bot
chore(release): 8.0.0 [skip ci]
235826a 
# [8.0.0](7.1.0...8.0.0) (2020-03-27)

### Bug Fixes

* broken template ([68d287e](68d287e))
* i18n for Electron ([15c7615](15c7615))
* i18n language imports ([0b8a1a6](0b8a1a6))
* ionic css import ([2a4b5cd](2a4b5cd))
* jest alias and update cases for prefix ([6f84b73](6f84b73))
* languages listing on windows ([37d5656](37d5656))
* material header not showing in cordova (close [#215](#215), [#511](#511)) ([2b78ff0](2b78ff0))
* misplaced prompt for puppeteer ([5cb4b60](5cb4b60))
* missing global mock when using Jest ([#509](#509)) ([32c69cf](32c69cf))
* stylelint issue ([451b47b](451b47b))
* template issues ([046fd48](046fd48))
* ts error with cordova ([9ae8b51](9ae8b51))
* webview config for ios/android (close [#491](#491)) ([79f4e49](79f4e49))

### Features

* add de-DE translation ([f08f6ec](f08f6ec))
* add deployment options (close [#540](#540), [#327](#327)) ([7602e8c](7602e8c))
* add i18n module and language selection at generation (close [#262](#262)) ([383d6fb](383d6fb))
* add prefix and aliasing for core/shared (close [#277](#277)) ([faa1981](faa1981))
* aggregate all auth related components in auth module (close [#535](#535)) ([8dbadb7](8dbadb7))
* make Angular Material the default UI for web apps ([626505c](626505c))
* remove HttpService and CacheService ([99eea62](99eea62))
* remove usage of puppeter by default for testing (fix [#522](#522)) ([b786c23](b786c23))
* update bootstrap template for ng9 ([88d3d58](88d3d58))
* update de-DE translation ([3c0b244](3c0b244))
* update to Ionic 5 ([589a1fb](589a1fb))
* upgrade to ng9 for material path ([97d8fb0](97d8fb0))

### BREAKING CHANGES

* core/shared folders now have an `@` prefix to have them listed first,
along with a direct TypeScript alias for imports.
* Removing HttpClient overload as this is definitely not something
the Angular team is looking to support. Also it's not relevant and
useful anymore thanks to state management libraries, that's why
CacheService is also removed.
@sinedied
Copy link
Member

🎉 This issue has been resolved in version 8.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
chore feature good first contribution PR welcome released
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@captaincaius @sinedied