yourealwaysbe / forkyz

Forkyz Crosswords
GNU General Public License v3.0
39 stars 5 forks source link

Fails to download puzzles on Android 12 #36

Closed tgiannak closed 2 years ago

tgiannak commented 2 years ago

Downloading puzzles fails on Android 12.

adb logcat shows the following error message for each puzzle source when attempting to download puzzles:

12-24 10:35:04.377 28521 28556 W yourealwaysbe: Failed to download Sheffer Crossword
12-24 10:35:04.377 28521 28556 W yourealwaysbe: java.lang.IllegalArgumentException: app.crossword.yourealwaysbe.forkyz: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
12-24 10:35:04.377 28521 28556 W yourealwaysbe: Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at android.app.PendingIntent.checkFlags(PendingIntent.java:375)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at android.app.PendingIntent.getActivityAsUser(PendingIntent.java:458)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at android.app.PendingIntent.getActivity(PendingIntent.java:444)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at android.app.PendingIntent.getActivity(PendingIntent.java:408)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at app.crossword.yourealwaysbe.net.Downloaders.downloadPuzzle(Downloaders.java:197)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at app.crossword.yourealwaysbe.net.Downloaders.download(Downloaders.java:158)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at app.crossword.yourealwaysbe.net.Downloaders.download(Downloaders.java:128)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at app.crossword.yourealwaysbe.BrowseActivityViewModel.lambda$download$6$BrowseActivityViewModel(BrowseActivityViewModel.java:242)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at app.crossword.yourealwaysbe.-$$Lambda$BrowseActivityViewModel$nn99MSBC6eaIWFbc0uHZnu3mlHs.run(Unknown Source:8)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
12-24 10:35:04.377 28521 28556 W yourealwaysbe:     at java.lang.Thread.run(Thread.java:920)
yourealwaysbe commented 2 years ago

Thanks for this. I've pushed a naive fix here 3e54911308db7bae5f39a05503e7190e72bcf7aa. I'm a bit surprised the linter didn't pick it up.

I haven't been able to test it -- my fault for targetting an Android version i don't have access to. My attempts to install an emulator fell flat. If you're able to test it, let me know. Otherwise i'll spend more time fighting with the emulator after Christmas.

tgiannak commented 2 years ago

I'm happy to help test.

While fixing the call to PendingIntent.getActivity on line 198 in Downloaders.downloadPuzzle appears to be enough to get the puzzles to download, there are several other calls to getActivity that seem to need to be updated as well.

The ones in Downloaders.postDownloadedGeneral and Downloaders.postDownloadedNotification have to be updated to get the download completed notifications. Setting those to PendingIntent.FLAG_IMMUTABLE seems to work.

What would I have to do to exercise the two calls in Scraper?

yourealwaysbe commented 2 years ago

While fixing the call to PendingIntent.getActivity on line 198 in Downloaders.downloadPuzzle appears to be enough to get the puzzles to download, there are several other calls to getActivity that seem to need to be updated as well.

Thanks, missed those somehow. I pushed again to the version15 branch, i think i got them all. Moved the flag to version utils because the flag doesn't exist below Android 23.

https://github.com/yourealwaysbe/forkyz/tree/version15

The ones in Downloaders.postDownloadedGeneral and Downloaders.postDownloadedNotification have to be updated to get the download completed notifications. Setting those to PendingIntent.FLAG_IMMUTABLE seems to work.

Great, thanks.

What would I have to do to exercise the two calls in Scraper?

In settings, enable one of the experimental sources (Keglars or Cryptic Cru) and download.

tgiannak commented 2 years ago

The version15 branch (as of eb7e18f) fails to download puzzles from both normal and experimental sources, with the same error message as before. I only see the version of the immutablePendingIntentFlag method that returns 0, in HoneycombUtil.

yourealwaysbe commented 2 years ago

Should be there now, sorry about that.

tgiannak commented 2 years ago

I think that OreoUtil needs to extend MarhsmallowUtil instead of LollipopUtil, as well. With that change it works (though one of the "Kegler's Kryptics" puzzles seems to be causing the app to crash when it displays in the list of puzzles--which is a separate problem).

yourealwaysbe commented 2 years ago

Ugh. I should do this when I'm actually paying attention. Thanks.

Which Keglar puzzle is it?

tgiannak commented 2 years ago

I can't easily tell, because the app crashes before I can see. It is one of the ones that is fetched when downloading the puzzles for today (2021-12-25). I believe it's a Keglar puzzle because it doesn't happen until I download them.

I'll do some investigation later and open a separate ticket once I figure out which one it is, since it seems to be unrelated to the issue with the PendingIntent API change.

Thank you for working on this!

yourealwaysbe commented 2 years ago

With that change it works (though one of the "Kegler's Kryptics" puzzles seems to be causing the app to crash when it displays in the list of puzzles--which is a separate problem).

I think i found this error yesterday and fixed it in 6a86a0579b6353f56d361fb0b4c6f88f383de40d. Doesn't look like an Android 12 issue, but i only ran into it while checking on a new Android 12 installation :)

Some of the Keglar and/or Cryptic Cru puzzles have *s in the author names, and i was constructing regexes from them without quoting.