Skip to content

Commit fed215a

Browse files
bvibberbvibber
committed
Crash fix for rotate in media detail view
We were crashing when rotating in the detail view, because the detail view fragment had assumed that its parent activity had already loaded up its cursor and such before it got created -- an assumption which fails dramatically when rotating, or switching away and back after we got paged out of memory. Check whether we got an object, and if not ask the parent to ping us when it's done loading. Also tweaked one of the older delay hacks to avoid a second crash related to the pager's adapter. Change-Id: Ib91c0fef0d8ffd0e6b8e0e4874f3ee9be3e9cf34
1 parent 0fb60f0 commit fed215a

File tree

4 files changed

+100
-64
lines changed

4 files changed

+100
-64
lines changed

‎commons/src/main/java/org/wikimedia/commons/contributions/ContributionsActivity.java

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.wikimedia.commons.contributions;
22

3+
import android.database.DataSetObserver;
34
import android.os.IBinder;
45
import android.support.v4.app.FragmentManager;
56
import android.support.v4.app.LoaderManager;
@@ -12,6 +13,7 @@
1213
import android.util.Log;
1314
import android.view.View;
1415
import android.widget.AdapterView;
16+
import android.widget.Adapter;
1517
import com.actionbarsherlock.view.Menu;
1618
import com.actionbarsherlock.view.MenuItem;
1719

@@ -36,6 +38,7 @@ public class ContributionsActivity
3638
private Cursor allContributions;
3739
private ContributionsListFragment contributionsList;
3840
private MediaDetailPagerFragment mediaDetails;
41+
private ArrayList<DataSetObserver> observersWaitingForLoad = new ArrayList<DataSetObserver>();
3942

4043
private Campaign campaign;
4144

@@ -226,6 +229,7 @@ public void onLoadFinished(Loader cursorLoader, Object result) {
226229
((MediaListAdapter)contributionsList.getAdapter()).updateMediaList((ArrayList<Media>) result);
227230
}
228231
}
232+
notifyAndClearDataSetObservers();
229233
}
230234

231235
public void onLoaderReset(Loader cursorLoader) {
@@ -237,7 +241,10 @@ public void onLoaderReset(Loader cursorLoader) {
237241
}
238242

239243
public Media getMediaAtPosition(int i) {
240-
if(campaign == null) {
244+
if (contributionsList.getAdapter() == null) {
245+
// not yet ready to return data
246+
return null;
247+
} else if(campaign == null) {
241248
return Contribution.fromCursor((Cursor) contributionsList.getAdapter().getItem(i));
242249
} else {
243250
return (Media) contributionsList.getAdapter().getItem(i);
@@ -255,6 +262,31 @@ public void notifyDatasetChanged() {
255262
// Do nothing for now
256263
}
257264

265+
private void notifyAndClearDataSetObservers() {
266+
for (DataSetObserver observer : observersWaitingForLoad) {
267+
observer.onChanged();
268+
}
269+
observersWaitingForLoad.clear();
270+
}
271+
272+
public void registerDataSetObserver(DataSetObserver observer) {
273+
Adapter adapter = contributionsList.getAdapter();
274+
if (adapter == null) {
275+
observersWaitingForLoad.add(observer);
276+
} else {
277+
adapter.registerDataSetObserver(observer);
278+
}
279+
}
280+
281+
public void unregisterDataSetObserver(DataSetObserver observer) {
282+
Adapter adapter = contributionsList.getAdapter();
283+
if (adapter == null) {
284+
observersWaitingForLoad.remove(observer);
285+
} else {
286+
adapter.registerDataSetObserver(observer);
287+
}
288+
}
289+
258290
public void onBackStackChanged() {
259291
if(mediaDetails != null && mediaDetails.isVisible()) {
260292
getSupportActionBar().setDisplayHomeAsUpEnabled(true);

‎commons/src/main/java/org/wikimedia/commons/media/MediaDetailFragment.java

Lines changed: 53 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.wikimedia.commons.media;
22

33
import android.content.Intent;
4+
import android.database.DataSetObserver;
45
import android.graphics.*;
56
import android.os.*;
67
import android.text.*;
@@ -62,6 +63,7 @@ public static MediaDetailFragment forMedia(int index, boolean editable) {
6263
private boolean categoriesPresent = false;
6364
private ViewTreeObserver.OnGlobalLayoutListener layoutListener; // for layout stuff, only used once!
6465
private ViewTreeObserver.OnScrollChangedListener scrollListener;
66+
DataSetObserver dataObserver;
6567
private AsyncTask<Void,Void,Boolean> detailFetchTask;
6668
private LicenseList licenseList;
6769

@@ -93,7 +95,6 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa
9395
index = getArguments().getInt("index");
9496
initialListTop = 0;
9597
}
96-
final Media media = detailProvider.getMediaAtPosition(index);
9798
categoryNames = new ArrayList<String>();
9899
categoryNames.add(getString(R.string.detail_panel_cats_loading));
99100

@@ -113,18 +114,60 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa
113114

114115
licenseList = new LicenseList(getActivity());
115116

116-
// Enable or disable editing on the title
117-
/*
118-
title.setClickable(editable);
119-
title.setFocusable(editable);
120-
title.setCursorVisible(editable);
121-
title.setFocusableInTouchMode(editable);
122-
if(!editable) {
123-
title.setBackgroundDrawable(null);
117+
Media media = detailProvider.getMediaAtPosition(index);
118+
if (media == null) {
119+
// Ask the detail provider to ping us when we're ready
120+
Log.d("Commons", "MediaDetailFragment not yet ready to display details; registering observer");
121+
dataObserver = new DataSetObserver() {
122+
public void onChanged() {
123+
Log.d("Commons", "MediaDetailFragment ready to display delayed details!");
124+
detailProvider.unregisterDataSetObserver(dataObserver);
125+
dataObserver = null;
126+
displayMediaDetails(detailProvider.getMediaAtPosition(index));
127+
}
128+
};
129+
detailProvider.registerDataSetObserver(dataObserver);
130+
} else {
131+
Log.d("Commons", "MediaDetailFragment ready to display details");
132+
displayMediaDetails(media);
124133
}
125-
*/
126134

135+
// Progressively darken the image in the background when we scroll detail pane up
136+
scrollListener = new ViewTreeObserver.OnScrollChangedListener() {
137+
public void onScrollChanged() {
138+
updateTheDarkness();
139+
}
140+
};
141+
view.getViewTreeObserver().addOnScrollChangedListener(scrollListener);
142+
143+
// Layout layoutListener to size the spacer item relative to the available space.
144+
// There may be a .... better way to do this.
145+
layoutListener = new ViewTreeObserver.OnGlobalLayoutListener() {
146+
private int currentHeight = -1;
147+
148+
public void onGlobalLayout() {
149+
int viewHeight = view.getHeight();
150+
//int textHeight = title.getLineHeight();
151+
int paddingDp = 112;
152+
float paddingPx = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, paddingDp, getResources().getDisplayMetrics());
153+
int newHeight = viewHeight - Math.round(paddingPx);
154+
155+
if (newHeight != currentHeight) {
156+
currentHeight = newHeight;
157+
ViewGroup.LayoutParams params = spacer.getLayoutParams();
158+
params.height = newHeight;
159+
spacer.setLayoutParams(params);
160+
161+
scrollView.scrollTo(0, initialListTop);
162+
}
163+
164+
}
165+
};
166+
view.getViewTreeObserver().addOnGlobalLayoutListener(layoutListener);
167+
return view;
168+
}
127169

170+
private void displayMediaDetails(final Media media) {
128171
String actualUrl = (media.getLocalUri() != null && TextUtils.isEmpty(media.getLocalUri().toString())) ? media.getLocalUri().toString() : media.getThumbnailUrl(640);
129172
if(actualUrl.startsWith("http")) {
130173
ImageLoader loader = ((CommonsApplication)getActivity().getApplicationContext()).getImageLoader();
@@ -212,58 +255,6 @@ public void onLoadingCancelled(String s, View view) {
212255
title.setText(media.getDisplayTitle());
213256
desc.setText(""); // fill in from network...
214257
license.setText(""); // fill in from network...
215-
216-
/*
217-
title.addTextChangedListener(new TextWatcher() {
218-
public void beforeTextChanged(CharSequence charSequence, int i, int i2, int i3) {
219-
220-
}
221-
222-
public void onTextChanged(CharSequence charSequence, int i, int i2, int i3) {
223-
detailProvider.getMediaAtPosition(index).setFilename(title.getText().toString());
224-
detailProvider.getMediaAtPosition(index).setTag("isDirty", true);
225-
detailProvider.notifyDatasetChanged();
226-
}
227-
228-
public void afterTextChanged(Editable editable) {
229-
230-
}
231-
});
232-
*/
233-
234-
// Progressively darken the image in the background when we scroll detail pane up
235-
scrollListener = new ViewTreeObserver.OnScrollChangedListener() {
236-
public void onScrollChanged() {
237-
updateTheDarkness();
238-
}
239-
};
240-
view.getViewTreeObserver().addOnScrollChangedListener(scrollListener);
241-
242-
// Layout layoutListener to size the spacer item relative to the available space.
243-
// There may be a .... better way to do this.
244-
layoutListener = new ViewTreeObserver.OnGlobalLayoutListener() {
245-
private int currentHeight = -1;
246-
247-
public void onGlobalLayout() {
248-
int viewHeight = view.getHeight();
249-
//int textHeight = title.getLineHeight();
250-
int paddingDp = 112;
251-
float paddingPx = TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, paddingDp, getResources().getDisplayMetrics());
252-
int newHeight = viewHeight - Math.round(paddingPx);
253-
254-
if (newHeight != currentHeight) {
255-
currentHeight = newHeight;
256-
ViewGroup.LayoutParams params = spacer.getLayoutParams();
257-
params.height = newHeight;
258-
spacer.setLayoutParams(params);
259-
260-
scrollView.scrollTo(0, initialListTop);
261-
}
262-
263-
}
264-
};
265-
view.getViewTreeObserver().addOnGlobalLayoutListener(layoutListener);
266-
return view;
267258
}
268259

269260
@Override

‎commons/src/main/java/org/wikimedia/commons/media/MediaDetailPagerFragment.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import android.app.DownloadManager;
44
import android.content.*;
55
import android.database.Cursor;
6+
import android.database.DataSetObserver;
67
import android.net.*;
78
import android.os.*;
89
import android.support.v4.app.Fragment;
@@ -41,6 +42,8 @@ public interface MediaDetailProvider {
4142
public Media getMediaAtPosition(int i);
4243
public int getTotalMediaCount();
4344
public void notifyDatasetChanged();
45+
public void registerDataSetObserver(DataSetObserver observer);
46+
public void unregisterDataSetObserver(DataSetObserver observer);
4447
}
4548
private class MediaDetailAdapter extends FragmentStatePagerAdapter {
4649

@@ -80,17 +83,19 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa
8083
View view = inflater.inflate(R.layout.fragment_media_detail_pager, container, false);
8184
pager = (ViewPager) view.findViewById(R.id.mediaDetailsPager);
8285
pager.setOnPageChangeListener(this);
83-
pager.setAdapter(new MediaDetailAdapter(getChildFragmentManager()));
8486
if(savedInstanceState != null) {
8587
final int pageNumber = savedInstanceState.getInt("current-page");
8688
// Adapter doesn't seem to be loading immediately.
8789
// Dear God, please forgive us for our sins
8890
view.postDelayed(new Runnable() {
8991
public void run() {
92+
pager.setAdapter(new MediaDetailAdapter(getChildFragmentManager()));
9093
pager.setCurrentItem(pageNumber, false);
9194
getSherlockActivity().supportInvalidateOptionsMenu();
9295
}
9396
}, 100);
97+
} else {
98+
pager.setAdapter(new MediaDetailAdapter(getChildFragmentManager()));
9499
}
95100
return view;
96101
}

‎commons/src/main/java/org/wikimedia/commons/upload/MultipleShareActivity.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import android.app.*;
77
import android.content.*;
8+
import android.database.DataSetObserver;
89
import android.net.*;
910
import android.os.*;
1011
import android.support.v4.app.FragmentManager;
@@ -61,6 +62,13 @@ public void notifyDatasetChanged() {
6162
}
6263
}
6364

65+
public void registerDataSetObserver(DataSetObserver observer) {
66+
// fixme implement me if needed
67+
}
68+
69+
public void unregisterDataSetObserver(DataSetObserver observer) {
70+
// fixme implement me if needed
71+
}
6472

6573
public void onItemClick(AdapterView<?> adapterView, View view, int index, long item) {
6674
showDetail(index);

0 commit comments

Comments
 (0)