Skip to content

Bugfix Media Item Delete Crash #13016

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

Merged
merged 11 commits into from
May 21, 2024
Merged

Bugfix Media Item Delete Crash #13016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0e2faf3
Use view pager 2 and delete image properly
alperozturk96 May 16, 2024
8a0ef76
Notify only deleted position
alperozturk96 May 16, 2024
8618a5a
Simplify onRemoteOperationFinish
alperozturk96 May 16, 2024
d47c5b8
Rename .java to .kt
alperozturk96 May 16, 2024
cd8e11b
Convert to kotlin and fix delete
alperozturk96 May 16, 2024
d667853
Optimize imports
alperozturk96 May 16, 2024
d1c2cc4
Remove unused CustomViewPager
alperozturk96 May 16, 2024
4e1dda3
Fix code analytics
alperozturk96 May 16, 2024
07e00f4
Fix scrolling in view pager.
ZetaTom May 17, 2024
11139e4
Reduce lint
alperozturk96 May 17, 2024
f94ad4e
Use non nullable parameters, delete unused functions
alperozturk96 May 21, 2024
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
47 changes: 0 additions & 47 deletions app/src/main/java/com/owncloud/android/ui/components/CustomViewPager.java
View file
Open in desktop

This file was deleted.

109 changes: 42 additions & 67 deletions app/src/main/java/com/owncloud/android/ui/preview/PreviewImageActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
import androidx.appcompat.app.ActionBar;
import androidx.drawerlayout.widget.DrawerLayout;
import androidx.localbroadcastmanager.content.LocalBroadcastManager;
import androidx.viewpager.widget.ViewPager;
import androidx.viewpager2.widget.ViewPager2;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;

/**
Expand All @@ -68,7 +68,6 @@
@SuppressWarnings("PMD.AvoidDuplicateLiterals")
public class PreviewImageActivity extends FileActivity implements
FileFragment.ContainerActivity,
ViewPager.OnPageChangeListener,
OnRemoteOperationListener,
Injectable {

Expand All @@ -78,13 +77,12 @@ public class PreviewImageActivity extends FileActivity implements
private static final String KEY_SYSTEM_VISIBLE = "TRUE";

private OCFile livePhotoFile;
private ViewPager viewPager;
private ViewPager2 viewPager;
private PreviewImagePagerAdapter previewImagePagerAdapter;
private int savedPosition;
private boolean hasSavedPosition;
private boolean requestWaitingForBinder;
private DownloadFinishReceiver downloadFinishReceiver;
private UploadFinishReceiver uploadFinishReceiver;
private View fullScreenAnchorView;
private boolean isDownloadWorkStarted = false;

Expand Down Expand Up @@ -158,7 +156,7 @@ private void initViewPager(User user) {
if (virtualFolderType != null && virtualFolderType != VirtualFolderType.NONE) {
VirtualFolderType type = (VirtualFolderType) virtualFolderType;

previewImagePagerAdapter = new PreviewImagePagerAdapter(getSupportFragmentManager(),
previewImagePagerAdapter = new PreviewImagePagerAdapter(this,
type,
user,
getStorageManager());
Expand All @@ -168,11 +166,11 @@ private void initViewPager(User user) {

if (parentFolder == null) {
// should not be necessary
parentFolder = getStorageManager().getFileByPath(OCFile.ROOT_PATH);
parentFolder = getStorageManager().getFileByEncryptedRemotePath(OCFile.ROOT_PATH);
}

previewImagePagerAdapter = new PreviewImagePagerAdapter(
getSupportFragmentManager(),
this,
livePhotoFile,
parentFolder,
user,
Expand All @@ -185,11 +183,16 @@ private void initViewPager(User user) {
viewPager = findViewById(R.id.fragmentPager);

int position = hasSavedPosition ? savedPosition : previewImagePagerAdapter.getFilePosition(getFile());
position = position >= 0 ? position : 0;
position = Math.max(position, 0);

viewPager.setAdapter(previewImagePagerAdapter);
viewPager.addOnPageChangeListener(this);
viewPager.setCurrentItem(position);
viewPager.registerOnPageChangeCallback(new ViewPager2.OnPageChangeCallback() {
@Override
public void onPageSelected(int position) {
selectPage(position);
}
});
viewPager.setCurrentItem(position, false);

if (position == 0 && !getFile().isDown()) {
// this is necessary because mViewPager.setCurrentItem(0) just after setting the
Expand Down Expand Up @@ -247,7 +250,7 @@ public void onStart() {
if (file != null) {
/// Refresh the activity according to the Account and OCFile set
setFile(file); // reset after getting it fresh from storageManager
getSupportActionBar().setTitle(getFile().getFileName());
updateActionBarTitle(getFile().getFileName());
//if (!stateWasRecovered) {
initViewPager(optionalUser.get());
//}
Expand All @@ -271,15 +274,16 @@ public void onRemoteOperationFinish(RemoteOperation operation, RemoteOperationRe
super.onRemoteOperationFinish(operation, result);

if (operation instanceof RemoveFileOperation) {
// initialize the pager with the new file list
initViewPager(getUser().get());
if (viewPager.getAdapter().getCount() > 0) {
// Trigger page reselection, to update the title
onPageSelected(viewPager.getCurrentItem());
} else {
// Last file has been deleted, so finish the activity
int deletePosition = viewPager.getCurrentItem();
int nextPosition = deletePosition > 0 ? deletePosition - 1 : 0;

if (previewImagePagerAdapter.getItemCount() <= 1) {
finish();
return;
}

viewPager.setCurrentItem(nextPosition, true);
previewImagePagerAdapter.delete(deletePosition);
} else if (operation instanceof SynchronizeFileOperation) {
onSynchronizeFileOperationFinish(result);
}
Expand All @@ -301,7 +305,7 @@ private void observeWorkerState() {
requestWaitingForBinder = false;
Log_OC.d(TAG, "Simulating reselection of current page after connection " +
"of download binder");
onPageSelected(viewPager.getCurrentItem());
selectPage(viewPager.getCurrentItem());
}
} else {
Log_OC.d(TAG, "Download worker stopped");
Expand All @@ -328,7 +332,7 @@ protected void onResume() {
IntentFilter downloadIntentFilter = new IntentFilter(FileDownloadWorker.Companion.getDownloadFinishMessage());
localBroadcastManager.registerReceiver(downloadFinishReceiver, downloadIntentFilter);

uploadFinishReceiver = new UploadFinishReceiver();
UploadFinishReceiver uploadFinishReceiver = new UploadFinishReceiver();
IntentFilter uploadIntentFilter = new IntentFilter(FileUploadWorker.Companion.getUploadFinishMessage());
localBroadcastManager.registerReceiver(uploadFinishReceiver, uploadIntentFilter);
}
Expand Down Expand Up @@ -384,8 +388,7 @@ public void requestForDownload(OCFile file, String downloadBehaviour) {
*
* @param position Position index of the new selected page
*/
@Override
public void onPageSelected(int position) {
public void selectPage(int position) {
savedPosition = position;
hasSavedPosition = true;

Expand All @@ -405,45 +408,21 @@ public void onPageSelected(int position) {
}
}

// Update ActionBar title
if (currentFile != null) {
if (getSupportActionBar() != null) {
getSupportActionBar().setTitle(currentFile.getFileName());
}
updateActionBarTitle(currentFile.getFileName());
setDrawerIndicatorEnabled(false);
}
}

/**
* Called when the scroll state changes. Useful for discovering when the user begins dragging,
* when the pager is automatically settling to the current page. when it is fully stopped/idle.
*
* @param state The new scroll state (SCROLL_STATE_IDLE, _DRAGGING, _SETTLING
*/
@Override
public void onPageScrollStateChanged(int state) {
// not used at the moment
}

/**
* This method will be invoked when the current page is scrolled, either as part of a
* programmatically initiated smooth scroll or a user initiated touch scroll.
*
* @param position Position index of the first page currently being displayed.
* Page position+1 will be visible if positionOffset is
* nonzero.
* @param positionOffset Value from [0, 1) indicating the offset from the page
* at position.
* @param positionOffsetPixels Value in pixels indicating the offset from position.
*/
@Override
public void onPageScrolled(int position, float positionOffset, int positionOffsetPixels) {
// not used at the moment
public void updateActionBarTitle(String title) {
if (getSupportActionBar() != null) {
getSupportActionBar().setTitle(title);
}
}

/**
* Class waiting for broadcast events from the {@link FileDownloadWorker} service.
*
* <p>
* Updates the UI when a download is started or finished, provided that it is relevant for the
* folder displayed in the gallery.
*/
Expand All @@ -465,8 +444,9 @@ private void previewNewImage(Intent intent) {
String accountName = intent.getStringExtra(FileDownloadWorker.EXTRA_ACCOUNT_NAME);
String downloadedRemotePath = intent.getStringExtra(FileDownloadWorker.EXTRA_REMOTE_PATH);
String downloadBehaviour = intent.getStringExtra(OCFileListFragment.DOWNLOAD_BEHAVIOUR);

if (getAccount().name.equals(accountName) && downloadedRemotePath != null) {
OCFile file = getStorageManager().getFileByPath(downloadedRemotePath);
OCFile file = getStorageManager().getFileByEncryptedRemotePath(downloadedRemotePath);
boolean downloadWasFine = intent.getBooleanExtra(FileDownloadWorker.EXTRA_DOWNLOAD_RESULT, false);

if (EditImageActivity.OPEN_IMAGE_EDITOR.equals(downloadBehaviour)) {
Expand All @@ -476,16 +456,19 @@ private void previewNewImage(Intent intent) {
if (position >= 0) {
if (downloadWasFine) {
previewImagePagerAdapter.updateFile(position, file);

} else {
previewImagePagerAdapter.updateWithDownloadError(position);
}
previewImagePagerAdapter.notifyDataSetChanged(); // will trigger the creation of new fragments
previewImagePagerAdapter.notifyItemChanged(position);
} else if (downloadWasFine) {
initViewPager(getUser().get());
int newPosition = previewImagePagerAdapter.getFilePosition(file);
if (newPosition >= 0) {
viewPager.setCurrentItem(newPosition);
Optional<User> user = getUser();

if (user.isPresent()) {
initViewPager(user.get());
int newPosition = previewImagePagerAdapter.getFilePosition(file);
if (newPosition >= 0) {
viewPager.setCurrentItem(newPosition);
}
}
}
}
Expand All @@ -502,19 +485,11 @@ public void toggleFullScreen() {

if (visible) {
hideSystemUI(fullScreenAnchorView);
// actionBar.hide(); // propagated through
// OnSystemUiVisibilityChangeListener()
} else {
showSystemUI(fullScreenAnchorView);
// actionBar.show(); // propagated through
// OnSystemUiVisibilityChangeListener()
}
}

public void switchToFullScreen() {
hideSystemUI(fullScreenAnchorView);
}

public void startImageEditor(OCFile file) {
if (file.isDown()) {
Intent editImageIntent = new Intent(this, EditImageActivity.class);
Expand Down
14 changes: 12 additions & 2 deletions app/src/main/java/com/owncloud/android/ui/preview/PreviewImageFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import androidx.fragment.app.FragmentManager;
import androidx.fragment.app.FragmentStatePagerAdapter;
import androidx.fragment.app.FragmentTransaction;
import androidx.viewpager2.adapter.FragmentStateAdapter;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import pl.droidsonroids.gif.GifDrawable;

Expand Down Expand Up @@ -127,7 +128,7 @@ public class PreviewImageFragment extends FileFragment implements Injectable {
* This method hides to client objects the need of doing the construction in two steps.
*
* @param imageFile An {@link OCFile} to preview as an image in the fragment
* @param ignoreFirstSavedState Flag to work around an unexpected behaviour of {@link FragmentStatePagerAdapter} ;
* @param ignoreFirstSavedState Flag to work around an unexpected behaviour of {@link FragmentStateAdapter} ;
* TODO better solution
*/
public static PreviewImageFragment newInstance(@NonNull OCFile imageFile,
Expand Down Expand Up @@ -232,8 +233,17 @@ public void onActivityCreated(Bundle savedInstanceState) {
if (savedInstanceState != null) {
if (!ignoreFirstSavedState) {
OCFile file = BundleExtensionsKt.getParcelableArgument(savedInstanceState, EXTRA_FILE, OCFile.class);
if (file == null) {
return;
}

setFile(file);
binding.image.setScale(Math.min(binding.image.getMaximumScale(), savedInstanceState.getFloat(EXTRA_ZOOM)));

try {
binding.image.setScale(Math.min(binding.image.getMaximumScale(), savedInstanceState.getFloat(EXTRA_ZOOM)));
} catch (IllegalArgumentException e) {
Log_OC.d(TAG, "Error caught at setScale: " + e);
}
} else {
ignoreFirstSavedState = false;
}
Expand Down
Loading