Fix support issue on API levels < 24

capacitor-community / text-to-speech

⚡️ Capacitor plugin for synthesizing speech from text.

MIT License

96 stars 27 forks source link

Fix support issue on API levels < 24 #124

Closed ecc521 closed 3 months ago

ecc521 commented 3 months ago

Comparator.comparing does not work below API level 24. This PR switches to use a lambda expression instead.

Pull request checklist

Please check if your PR fulfills the following requirements:

[x] The changes have been tested successfully.

robingenz commented 3 months ago

This change was made by https://github.com/capacitor-community/text-to-speech/commit/ada706ed26ea99e8d3a6bc66d509b0528a6203c3 to fix #112.

ecc521 commented 3 months ago

So the Comparator.comparing code causes an application crash below API level 24 (so Android 5.1), which is still supported by Capacitor. This hardly seems desirable.

I’m a little unsure what the difference is in terms of how Comparator.comparing doesn’t result in the contract violation thing.

The lazy way to “fix” this would be to simply use the default voice below API level 24 to avoid a crash. Seems like we should actually address the problem though.

Switching from hashCode to a different ordering scheme is probably best. It would be a fully backwards compatible change as well.

robingenz commented 3 months ago

Feel free to update your PR and I will take a look.

ecc521 commented 3 months ago

Updated. Successfully tested these changes on Kindle Fire (API level 22) and OnePlus 7T (I think 34). The crash on API levels 21-23 is fixed and the plugin works correctly.

I cannot guarantee that #112 does not re-emerge in this PR (not sure how to repro the original issue), but since getName() is guaranteed to be unique and String.compareTo is being used (which is very well tested), I find it unlikely the problem re-energes.