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.

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

Fix support issue on API levels < 24 #124

Merged
merged 2 commits into from
Jun 22, 2024

Conversation

ecc521
Copy link
Contributor

@ecc521 ecc521 commented Jun 21, 2024

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:

  • The changes have been tested successfully.

Comparator.comparing does not work below API level 24
@robingenz
Copy link
Member

This change was made by ada706e to fix #112.

@ecc521
Copy link
Contributor Author

ecc521 commented Jun 21, 2024

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
Copy link
Member

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

…evel 24

getName() is guaranteed to be unique, hashCode is not, so this may fix potential issues with the use of hashCode. Also switch from using Comparator.comparing to allow for support on API levels 21-24
@ecc521 ecc521 marked this pull request as ready for review June 22, 2024 16:27
@ecc521
Copy link
Contributor Author

ecc521 commented Jun 22, 2024

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.

Copy link
Member

@robingenz robingenz left a comment

Choose a reason for hiding this comment

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

Let's give it a try!

@robingenz robingenz added platform: android Android platform bug/fix Something isn't working labels Jun 22, 2024
@robingenz robingenz merged commit 0ccace8 into capacitor-community:master Jun 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/fix Something isn't working platform: android Android platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants