Another fix for a memory leak. This one is not as big as #4150, but
still worth fixing for memory constrained devices imo.
The `DrawerImageLoader` implementation (a global singleton) references a
member of the `MainActivity`, causing the whole activity to leak.
This weird construct was introduced in #1989 to fix a crash, but I think
since we migrated to coroutines it is no longer necessary because all
calls get correctly cancelled. I tried reproducing the crash but could
not, so I'm pretty sure it is fine. I would appreciate it if someone
else could try it as well though.
(The crash could be reproduced on slow internet, when
`onFetchUserInfoSuccess` was called while the activity was being
destroyed, causing Glide to crash the app because it can't use destroyed
activities. `onFetchUserInfoSuccess` is now no longer called in this
case because it is inside a `lifecycleScope.launch` block.)
Found with Leak canary: The transformation ends up in Glide's memory
cache and leaks whole Activities through the view -> context reference.
This fixes the problem by removing the background detection logic (so
the view reference is no longer needed) and setting the background
directly instead. Looks exactly as before.
Currently translated at 100.0% (634 of 634 strings)
Co-authored-by: fin-w <puf@users.noreply.weblate.tusky.app>
Translate-URL: https://weblate.tusky.app/projects/tusky/tusky/cy/
Translation: Tusky/Tusky
This changes one word in the string `pref_failed_to_sync` in the file
`values/strings.xml`.
See my reasoning here #4133
"Failed to sync settings" changes to "Failed to sync preferences".
Currently translated at 100.0% (634 of 634 strings)
Translated using Weblate (Icelandic)
Currently translated at 100.0% (634 of 634 strings)
Co-authored-by: Sveinn í Felli <sv1@fellsnet.is>
Translate-URL: https://weblate.tusky.app/projects/tusky/tusky/is/
Translation: Tusky/Tusky
Steps to reproduce: Open the dialog to set a catption on an image.
Rotate the screen.
<details>
<summary>Stacktrace</summary>
```
Exception java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.TextView.setText(java.lang.CharSequence)' on a null object reference
at com.keylesspalace.tusky.components.compose.dialog.CaptionDialog.onCreateView (CaptionDialog.kt:61)
at androidx.fragment.app.Fragment.performCreateView (Fragment.java:3114)
at androidx.fragment.app.DialogFragment.performCreateView (DialogFragment.java:775)
at androidx.fragment.app.FragmentStateManager.createView (FragmentStateManager.java:557)
at androidx.fragment.app.FragmentStateManager.moveToExpectedState (FragmentStateManager.java:272)
at androidx.fragment.app.FragmentStore.moveToExpectedState (FragmentStore.java:114)
at androidx.fragment.app.FragmentManager.moveToState (FragmentManager.java:1455)
at androidx.fragment.app.FragmentManager.dispatchStateChange (FragmentManager.java:3034)
at androidx.fragment.app.FragmentManager.dispatchActivityCreated (FragmentManager.java:2952)
at androidx.fragment.app.FragmentController.dispatchActivityCreated (FragmentController.java:263)
at androidx.fragment.app.FragmentActivity.onStart (FragmentActivity.java:350)
at androidx.appcompat.app.AppCompatActivity.onStart (AppCompatActivity.java:251)
at android.app.Instrumentation.callActivityOnStart (Instrumentation.java:1543)
at android.app.Activity.performStart (Activity.java:8682)
at android.app.ActivityThread.handleStartActivity (ActivityThread.java:4219)
at android.app.servertransaction.TransactionExecutor.performLifecycleSequence (TransactionExecutor.java:221)
at android.app.servertransaction.TransactionExecutor.cycleToPath (TransactionExecutor.java:201)
at android.app.servertransaction.TransactionExecutor.executeLifecycleState (TransactionExecutor.java:173)
at android.app.servertransaction.TransactionExecutor.execute (TransactionExecutor.java:97)
at android.app.ActivityThread$H.handleMessage (ActivityThread.java:2584)
at android.os.Handler.dispatchMessage (Handler.java:106)
at android.os.Looper.loopOnce (Looper.java:226)
at android.os.Looper.loop (Looper.java:313)
at android.app.ActivityThread.main (ActivityThread.java:8810)
at java.lang.reflect.Method.invoke
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:604)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1067)
```
</details>
Restoring the saved caption after the view was created fixes the
problem.
When viewing a video in Tusky, there is a top toolbar where the
description is shown and the bottom toolbar where play, forward,
backward, and scrub controls are found. In both Tusky 23 and the new
media3 video player code, the logic for showing these toolbars is
*unrelated*; Tusky catches tap events and shows and hides the
description, and the Android media library separately catches tap events
and shows and hides the bottom toolbar. Meanwhile, Tusky and the Android
media library each separately manage a set of logic for auto-hiding
their respective toolbars after a certain number of seconds has passed.
This all results in several problems:
- The top and bottom toolbars can desync, so that one is visible and the
other is not, and tapping to show/hide after this will only swap which
one is visible. This happens *every* time you switch to another
application then back to Tusky while the video player is up.
- You can also desync the top and bottom toolbars in this way by simply
tapping very rapidly.
- The autohide logic was difficult for us to control or customize,
because it was partially hidden inside the Android libraries (relevant
because under media3, the autohide delay increased from 3 to something
like 5 or 6 seconds).
In this patch, I disabled all auto- and tap-based show/hide logic in
media3 and set the Tusky-side show/hide to directly control the media3
toolbar. I then audited the code with printfs until I understood the
state machine of show/hide, and removed anything irrational (some code
was either unreachable, or redundant; either these lines were broken in
the media3 transition, or they never worked).¹
While doing this, I made two policy changes:
- As discussed on Matrix, the autohide delay is now 4 seconds. (In
discussions with users on Mastodon, some complained the previous 3
seconds was too short; but in my opinion and [I think?] charlag's, the
new 5 seconds is too long).
- In the pre-existing code, if the user has hidden the controls, and
they switch to another app and back, the controls display for 4 seconds
then re-hide themselves, just like if the video had been presented for
the first time. I think this is good and kept it— *however* I made a
decision if the user intentionally taps to display the controls, *then*
switches to another app and back, the controls should *not* auto-hide,
because the user most recently requested those controls be shown.
Tests I performed on the final PR (successfully):
- Start video. Expect: toolbar+description hides after 4 seconds.
- Start video. Pause. Resume. Expect: t+d hides after 4 seconds.
- Start video. Wait 4 seconds until t+d hide. Switch to other app.
Switch back. Expect: t+d reappears, then hides after 4 seconds.
- Start video. Wait 4 seconds until t+d hide. Tap to show t+d. Switch to
other app. Switch back. Expect: t+d appear, do NOT autohide.
- Start video. Before 4 seconds up, switch to other app. Switch back.
Expect: t+d reappears, then hides after 4 seconds.
- Start video. Pause. Resume. Before 4 seconds up, switch to other app.
Switch back. Expect: t+d reappears, then hides after 4 seconds.
- Start video. Wait 4 seconds until t+d hide. Tap rapidly over and over
for many seconds. Expect: Nothing weird
- Start *audio*. Expect: At no point does controller autohide, not even
if I switch to another app and back, but I can hide it by manually
tapping
These tests were performed on Android 13. There is an entirely separate
`Build.VERSION.SDK_INT <= 23` path I did not test, but Android Studio
says this is dead code (I think it thinks our minimum SDK is higher than
that?)
---
<small>¹ Incidentally, the underlying cause of #4073 (the show/resume
part of it anyway) turned out to be that the STATE_READY event was being
received not just on video load but also a second time on app resume,
causing certain parts of the initialization code to run a second time
although the fragment had already been fully initialized.</small>
Ricardo
mcc
Konrad Pozniak
Ihor Hordiichuk
XoseM
fin-w
Luna Jernberg
André Ferreira
Hồ Nhất Duy
Chaman Vétéran
Bruno Miguel
TAKAHASHI Shuuji
Conny Duck
fin-w
Ümit Solmaz
Deleted User
Anatoly Bogomolov
puf
Sveinn í Felli
Kalle Kniivilä
Goooler
Levi Bard
UlrichKu
