Skip to content

Fix: Prevent crash when pausing/resuming uploads after screen rotation #6532

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+84 −56
Open

Fix: Prevent crash when pausing/resuming uploads after screen rotation #6532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8352bc5
fix:gracefully handle the missing presenter during configuration change
Kota-Jagadeesh Oct 19, 2025
38cd528
fix:safely retrieve fragments to prevent stale references after rotation
Kota-Jagadeesh Oct 19, 2025
33d3e44
Merge branch 'commons-app:main' into fix/upload-rotation-crash
Kota-Jagadeesh Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 52 additions & 43 deletions app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ class PendingUploadsFragment :
CommonsDaggerSupportFragment(),
PendingUploadsContract.View,
PendingUploadsAdapter.Callback {

//fix:public and nullable to allow the dagger injection and prevent crash on the rotation.
@Inject
lateinit var pendingUploadsPresenter: PendingUploadsPresenter
@JvmField
var pendingUploadsPresenter: PendingUploadsPresenter? = null

private lateinit var binding: FragmentPendingUploadsBinding

Expand All @@ -55,15 +58,11 @@ class PendingUploadsFragment :
): View {
super.onCreate(savedInstanceState)
binding = FragmentPendingUploadsBinding.inflate(inflater, container, false)
pendingUploadsPresenter.onAttachView(this)
pendingUploadsPresenter?.onAttachView(this)
initAdapter()
return binding.root
}

fun initAdapter() {
adapter = PendingUploadsAdapter(this)
}

override fun onViewCreated(
view: View,
savedInstanceState: Bundle?,
Expand All @@ -72,47 +71,51 @@ class PendingUploadsFragment :
initRecyclerView()
}

fun initAdapter() {
adapter = PendingUploadsAdapter(this)
}

/**
* Initializes the recycler view.
*/
private fun initRecyclerView() {
binding.pendingUploadsRecyclerView.setLayoutManager(LinearLayoutManager(this.context))
binding.pendingUploadsRecyclerView.adapter = adapter
pendingUploadsPresenter.setup()
pendingUploadsPresenter.totalContributionList
.observe(viewLifecycleOwner) { list: PagedList<Contribution> ->
contributionsSize = list.size
contributionsList = mutableListOf()
var pausedOrQueuedUploads = 0
list.forEach {
if (it != null) {
if (it.state == STATE_PAUSED ||
it.state == STATE_QUEUED ||
it.state == STATE_IN_PROGRESS
) {
contributionsList.add(it)
}
if (it.state == STATE_PAUSED || it.state == STATE_QUEUED) {
pausedOrQueuedUploads++
pendingUploadsPresenter?.setup()
pendingUploadsPresenter?.totalContributionList
?.observe(viewLifecycleOwner) { list: PagedList<Contribution> ->
contributionsSize = list.size
contributionsList = mutableListOf()
var pausedOrQueuedUploads = 0
list.forEach {
if (it != null) {
if (it.state == STATE_PAUSED ||
it.state == STATE_QUEUED ||
it.state == STATE_IN_PROGRESS
) {
contributionsList.add(it)
}
if (it.state == STATE_PAUSED || it.state == STATE_QUEUED) {
pausedOrQueuedUploads++
}
}
}
}
if (contributionsSize == 0) {
binding.nopendingTextView.visibility = View.VISIBLE
binding.pendingUplaodsLl.visibility = View.GONE
uploadProgressActivity.hidePendingIcons()
} else {
binding.nopendingTextView.visibility = View.GONE
binding.pendingUplaodsLl.visibility = View.VISIBLE
adapter.submitList(list)
binding.progressTextView.setText("$contributionsSize uploads left")
if ((pausedOrQueuedUploads == contributionsSize) || CommonsApplication.isPaused) {
uploadProgressActivity.setPausedIcon(true)
if (contributionsSize == 0) {
binding.nopendingTextView.visibility = View.VISIBLE
binding.pendingUplaodsLl.visibility = View.GONE
uploadProgressActivity.hidePendingIcons()
} else {
uploadProgressActivity.setPausedIcon(false)
binding.nopendingTextView.visibility = View.GONE
binding.pendingUplaodsLl.visibility = View.VISIBLE
adapter.submitList(list)
binding.progressTextView.setText("$contributionsSize uploads left")
if ((pausedOrQueuedUploads == contributionsSize) || CommonsApplication.isPaused) {
uploadProgressActivity.setPausedIcon(true)
} else {
uploadProgressActivity.setPausedIcon(false)
}
}
}
}
}

/**
Expand All @@ -129,7 +132,8 @@ class PendingUploadsFragment :
String.format(locale, activity.getString(R.string.no)),
{
ViewUtil.showShortToast(context, R.string.cancelling_upload)
pendingUploadsPresenter.deleteUpload(
// uses the safe call directly
pendingUploadsPresenter?.deleteUpload(
contribution, requireContext().applicationContext,
)
},
Expand All @@ -140,19 +144,24 @@ class PendingUploadsFragment :
/**
* Restarts all the paused uploads.
*/
fun restartUploads() = pendingUploadsPresenter.restartUploads(
contributionsList, 0, requireContext().applicationContext
)
fun restartUploads() {
pendingUploadsPresenter?.restartUploads(
contributionsList, 0, requireContext().applicationContext
)
}

/**
* Pauses all the ongoing uploads.
*/
fun pauseUploads() = pendingUploadsPresenter.pauseUploads()
fun pauseUploads() { pendingUploadsPresenter?.pauseUploads()
}

/**
* Cancels all the uploads after getting a confirmation from the user using Dialog.
*/
fun deleteUploads() {
pendingUploadsPresenter ?: return

val activity = requireActivity()
val locale = Locale.getDefault()
showAlertDialog(
Expand All @@ -164,7 +173,7 @@ class PendingUploadsFragment :
{
ViewUtil.showShortToast(context, R.string.cancelling_upload)
uploadProgressActivity.hidePendingIcons()
pendingUploadsPresenter.deleteUploads(
pendingUploadsPresenter?.deleteUploads(
listOf(
STATE_QUEUED,
STATE_IN_PROGRESS,
Expand All @@ -175,4 +184,4 @@ class PendingUploadsFragment :
{},
)
}
}
}
45 changes: 32 additions & 13 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadProgressActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import javax.inject.Inject
*/
class UploadProgressActivity : BaseActivity() {
private lateinit var binding: ActivityUploadProgressBinding
private var pendingUploadsFragment: PendingUploadsFragment? = null
private var failedUploadsFragment: FailedUploadsFragment? = null
var viewPagerAdapter: ViewPagerAdapter? = null
var menu: Menu? = null

Expand Down Expand Up @@ -68,18 +66,37 @@ class UploadProgressActivity : BaseActivity() {
setTabs()
}

private fun getPendingUploadsFragment(): PendingUploadsFragment? {
return supportFragmentManager.findFragmentByTag(
"android:switcher:${R.id.upload_progress_view_pager}:${0}",
) as? PendingUploadsFragment
}


// the helper to retrieve the current, non-stale FailedUploadsFragment instance.

private fun getFailedUploadsFragment(): FailedUploadsFragment? {
return supportFragmentManager.findFragmentByTag(
"android:switcher:${R.id.upload_progress_view_pager}:${1}",
) as? FailedUploadsFragment
}

/**
* Initializes and sets up the tabs data by creating instances of `PendingUploadsFragment`
* and `FailedUploadsFragment`, adds them to the `fragmentList`, and assigns corresponding
* titles from resources to the `titleList`.
*/
fun setTabs() {
pendingUploadsFragment = PendingUploadsFragment()
failedUploadsFragment = FailedUploadsFragment()
val pendingUploadsFragment: Fragment
val failedUploadsFragment: Fragment

// checks if the fragmentManager already has the fragments (after the rotation)
pendingUploadsFragment = getPendingUploadsFragment() ?: PendingUploadsFragment()
failedUploadsFragment = getFailedUploadsFragment() ?: FailedUploadsFragment()

viewPagerAdapter!!.setTabs(
R.string.pending to pendingUploadsFragment!!,
R.string.failed to failedUploadsFragment!!
R.string.pending to pendingUploadsFragment,
R.string.failed to failedUploadsFragment
)
viewPagerAdapter!!.notifyDataSetChanged()
}
Expand Down Expand Up @@ -119,7 +136,8 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.pause),
).setIcon(R.drawable.pause_icon)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.pauseUploads()
//retrieves the current fragment instance just before the use
getPendingUploadsFragment()?.pauseUploads()
setPausedIcon(true)
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
Expand All @@ -133,7 +151,8 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.cancel),
).setIcon(R.drawable.ic_cancel_upload)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.deleteUploads()
//retrieve the current fragment instance just before thee use
getPendingUploadsFragment()?.deleteUploads()
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
}
Expand All @@ -147,7 +166,8 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.resume),
).setIcon(R.drawable.play_icon)
.setOnMenuItemClickListener {
pendingUploadsFragment!!.restartUploads()
//retrieve the current fragment instance just before the use
getPendingUploadsFragment()?.restartUploads()
setPausedIcon(false)
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
Expand All @@ -160,8 +180,7 @@ class UploadProgressActivity : BaseActivity() {
menu!!
.add(Menu.NONE, R.id.retry_icon, Menu.NONE, getString(R.string.retry))
.setIcon(R.drawable.ic_refresh_24dp)
.setOnMenuItemClickListener {
failedUploadsFragment!!.restartUploads()
.setOnMenuItemClickListener { getFailedUploadsFragment()?.restartUploads()
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
}
Expand All @@ -174,7 +193,7 @@ class UploadProgressActivity : BaseActivity() {
getString(R.string.cancel),
).setIcon(R.drawable.ic_cancel_upload)
.setOnMenuItemClickListener {
failedUploadsFragment!!.deleteUploads()
getFailedUploadsFragment()?.deleteUploads()
true
}.setShowAsAction(MenuItem.SHOW_AS_ACTION_IF_ROOM)
}
Expand Down Expand Up @@ -208,4 +227,4 @@ class UploadProgressActivity : BaseActivity() {
isErrorIconsVisisble = visible
updateMenuItems(binding.uploadProgressViewPager.currentItem)
}
}
}