bug: phonegap-launch-navigator plugin navigate method stopped working after migrating to capacitor 4

[ryaa]

Bug Report

Capacitor Version

💊   Capacitor Doctor  💊

Latest Dependencies:

  @capacitor/cli: 4.3.0
  @capacitor/core: 4.3.0
  @capacitor/android: 4.3.0
  @capacitor/ios: 4.3.0

Installed Dependencies:

  @capacitor/cli: 4.3.0
  @capacitor/core: 4.3.0
  @capacitor/ios: 4.3.0
  @capacitor/android: 4.3.0

[success] iOS looking great! 👌
[success] Android looking great! 👌

Platform(s)

Android

Current Behavior

Invoking phonegap-launch-navigator plugin navigate method with an address param (start param - see https://github.com/dpa99c/phonegap-launch-navigator#navigate-to-a-destination-with-specified-start-location) started to always fail after migration to Capacitor 4. The problem is that the plugin tries to geocoder the address to coordinates (see https://github.com/dpa99c/phonegap-launch-navigator/blob/master/src/android/lib/LaunchNavigator.java#L1698) by sending http request and it always fails with android.os.NetworkOnMainThreadException because the plugin method is executed on the main thread.

My assumption is that this is because in Capacitor 4 there was the change made which executes postMessage on the main thread (https://github.com/ionic-team/capacitor/pull/5427/files#diff-7d14ff84aa5a03137668f3f22aa00981610317e3afbf5dcee8bb2c41bb31591bR30). The problem does not exist with Capacitor 3 so my assumption is that this is kind of a breaking change, which might make some plugins stop working

Expected Behavior

The existing plugins must work as before after migration to Capacitor 4

Code Reproduction

I hope that the above description is self explanatory however, i can create the required github repository showing the issue.

Additional Context

N/A

[Ionitron]

This issue may need more information before it can be addressed. In particular, it will need a reliable Code Reproduction that demonstrates the issue.

Please see the Contributing Guide for how to create a Code Reproduction.

Thanks!
Ionitron 💙

[ryaa]

Here is the repository with the app demonstrating the problem - see https://github.com/ryaa/launch-navigator-plugin-navigate-not-working

When you run the app, please tap the button on the main page. It will trigger an error at this line in the plugin https://github.com/dpa99c/phonegap-launch-navigator/blob/master/src/android/lib/LaunchNavigator.java#L344 (please also see the screenshot below) and log the error in the browser/webkit console

The problem is 100% reproducible.

[jcesarmobile]

Thanks for the sample app, I've been able to reproduce.
Calling postMessage in a background thread like this fixes the issue, but not sure if adding that now could be considered breaking.

bridge.execute(() -> {
    postMessage(message.getData());
});

We have a feature request for allowing to configure the bridge so people can keep using the old one, that should solve this issue too for now
#5949

[ryaa]

Calling postMessage in a background thread like this fixes the issue, but not sure if adding that now could be considered breaking.

bridge.execute(() -> {
    postMessage(message.getData());
});

I confirm that the above change fixes the problem for phonegap-launch-navigator plugin navigate method and does not break any other plugin functionalities. Questions - is there any specific reason why it was changed to make the plugin methods to be executed on the main thread? Do you think that it would be better to change to execute plugin methods on a different/not the main thread? Do you think that this change will be done in future Capacitor releases?

Thank you very much for a very prompt response on the above.

[jcesarmobile]

We changed the bridge to use a new bridge provided by google, it's the new bridge what executes code in the main thread while the old one did it in a background thread (because the call came from js and calls from js to native use a background thread).
It was google's decision to change the bridge to use the ui thread, to be honest, I didn't even realized until you brought this issue, because all our core plugins kept working with the new bridge. Not sure what's their reasoning for doing that or if it would be a good idea to use a background thread where google wants to use the ui thread.

We will discuss internally, but options are
a) provide a config option to use the old bridge
b) use background thread only for Cordova plugins
c) use background thread for all plugins (potentially breaking change)

a and b can happen in a minor or patch releases respectively, c might need to wait for a major release, but both a and b should fix this problem

[ionitron-bot]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Capacitor, please create a new issue and ensure the template is fully filled out.