Fix: No prompt to enable notifications #603
Conversation
| .build().show(getSupportFragmentManager(), null); | ||
| registerForActivityResult(new ActivityResultContracts.StartActivityForResult(), result -> { | ||
| if (result.getResultCode() == Activity.RESULT_OK) { | ||
| // TODO (aanorbel) Notify Users of successful completion |
There was a problem hiding this comment.
It seems we need to do this before merging
|
While trying to figure out whether this branch is WAI, I noticed I could not interrupt a running test and saw OnBackInvokedCallback is not enabled for the application. Set 'android:enableOnBackInvokedCallback="true"' in the application manifest.This warning or error inside the logcat. I also saw this warning: Found duplicated key: "experimental". This can cause unintended behaviour, please use unique keys for every preference.In any case, it's quite difficult to test this diff, as I cannot interrupt a background test that is taking $forever to finish. What's strange is that I see the test terminating earlier in the logs: {"key":"task_terminated","value":{}}The test is dnscheck and I have not enabled running long tests in the foreground. In any case, the app is now completely stuck and killing it does not fix it. I am going to reinstall and try again to see if I can reproduce this stuck behavior. Unfortunately, I was just casually using the app, so I don't have precise notes. Okay, this is crazy. I removed the app and reinstalled it, and it says there's still a test in progress. 😕 Going on memory, what I did is that I got bored of waiting for automated tests to finish, so I tried interrupting them and they were probably running inside dnscheck, because this is the last MK_EVENT entry I see in the logs. Okay, eventually the app started Web Connectivity but it's unclear to me the sequence of events. 🤔 |
The above warnings are not mainly linked to this PR. The First is due to the introduction of new API's to the project—updates by the Android team to the activity behaviour. The second is a result of a duplicate experimental key in the preference file. |
|
Stopping a test is a problem which I have planned to investigate since there have been a few complaints form @hellais as well as a previously reported issue. |
Yeah, I was also leaning towards thinking this.
Okay, good to see we were already aware of this issue. FWIW, I tried a bunch of times and could not provoke this issue again. I will try harder later today, as I continue to test this branch. Thank you! |
app/src/main/java/org/openobservatory/ooniprobe/activity/PromptActivity.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/openobservatory/ooniprobe/activity/PromptActivity.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/openobservatory/ooniprobe/activity/PromptActivity.java
Outdated
Show resolved
Hide resolved
|
I am always a bit lost when reading through Java code because classes connect in a two-dimensional way and I lack a bit of mental model about how the fundamental types connect in Android. I'll ask you to provide me with a walk through of the code and hopefully I will learn something that will stick. As regards the behavior, I think this diff is WAI, for what I could tell by testing the app on my phone. (Also I realized I messed up a bit with the review and all the comments should have been sent in a single batch, sorry for doing this! 🙏 ) |
Fixes ooni/probe#2507
Proposed Changes
Sounds greatis clicked.