jenkins-bot has submitted this change and it was merged.
Change subject: Fix ability to share text selection after screen rotation.
......................................................................
Fix ability to share text selection after screen rotation.
Refactored the way we initialize and clean up our ShareAdapters (both the
TextSelected and NoTextSelected adapters).
They were being instantiated in-place, and were creating Dialog objects
that weren't being destroyed properly upon screen rotation (leaking the
window). This was also causing an imbalance in the adding and removing of
the Clipboard Listener that receives the shared text from the WebView,
which was preventing the text from being shared after rotation.
I modified the ShareAdapters to inherit from the base ShareHandler, which
will maintain a reference to the Dialog that it instantiates, so that it
can be explicitly dismissed when necessary. And the Clipboard listener
will now persist for the lifetime of the Activity.
Bug: T89979
Change-Id: Ib081a48ef342c0844fa996c94314d41b6ff9d298
---
M wikipedia/src/main/java/org/wikipedia/page/PageActivity.java
M
wikipedia/src/main/java/org/wikipedia/page/snippet/NoTextSelectedShareAdapter.java
M wikipedia/src/main/java/org/wikipedia/page/snippet/ShareHandler.java
M
wikipedia/src/main/java/org/wikipedia/page/snippet/TextSelectedShareAdapter.java
4 files changed, 91 insertions(+), 74 deletions(-)
Approvals:
BearND: Looks good to me, approved
jenkins-bot: Verified
diff --git a/wikipedia/src/main/java/org/wikipedia/page/PageActivity.java
b/wikipedia/src/main/java/org/wikipedia/page/PageActivity.java
index dd13998..4eb076a 100644
--- a/wikipedia/src/main/java/org/wikipedia/page/PageActivity.java
+++ b/wikipedia/src/main/java/org/wikipedia/page/PageActivity.java
@@ -89,6 +89,7 @@
private View toolbarContainer;
private TextSelectedShareAdapter textSelectedShareAdapter;
+ private NoTextSelectedShareAdapter noTextSelectedShareAdapter;
private ActionMode currentActionMode;
private ActionBarDrawerToggle mDrawerToggle;
@@ -180,6 +181,11 @@
getSupportActionBar().setTitle("");
searchBarHideHandler = new SearchBarHideHandler(toolbarContainer);
+
+ noTextSelectedShareAdapter = new NoTextSelectedShareAdapter(this);
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB) {
+ textSelectedShareAdapter = new TextSelectedShareAdapter(this);
+ }
// TODO: remove this when we drop support for API 10
boolean themeChanged = false;
@@ -589,8 +595,8 @@
@Override
public void onBackPressed() {
- if (textSelectedShareAdapter != null) {
- textSelectedShareAdapter.finish();
+ if (currentActionMode != null) {
+ currentActionMode.finish();
return;
}
if (drawerLayout.isDrawerOpen(Gravity.START)) {
@@ -809,6 +815,10 @@
if (themeChooser != null && themeChooser.isShowing()) {
themeChooser.dismiss();
}
+ if (textSelectedShareAdapter != null) {
+ textSelectedShareAdapter.onStop();
+ }
+ noTextSelectedShareAdapter.onStop();
app.getSessionFunnel().persistSession();
@@ -833,9 +843,8 @@
// Saved Pages, or Find In Page). Otherwise, it must be the default
WebView text-
// highlighting ActionMode, in which case we'll invoke the Share
adapter!
if (WikipediaApp.getInstance().getReleaseType() ==
WikipediaApp.RELEASE_ALPHA
- && textSelectedShareAdapter == null
- && mode.getTag() == null) {
- textSelectedShareAdapter = new TextSelectedShareAdapter(this);
+ && mode.getTag() == null
+ && textSelectedShareAdapter != null) {
textSelectedShareAdapter.onTextSelected(mode);
}
super.onSupportActionModeStarted(mode);
@@ -843,14 +852,13 @@
@Override
public void onSupportActionModeFinished(ActionMode mode) {
- textSelectedShareAdapter = null;
currentActionMode = null;
super.onSupportActionModeFinished(mode);
}
public void share(PageTitle title) {
if (WikipediaApp.getInstance().getReleaseType() ==
WikipediaApp.RELEASE_ALPHA) {
- new NoTextSelectedShareAdapter(this).share();
+ noTextSelectedShareAdapter.share();
} else {
// TODO: remove once above block is promoted to production
Intent shareIntent = new Intent();
diff --git
a/wikipedia/src/main/java/org/wikipedia/page/snippet/NoTextSelectedShareAdapter.java
b/wikipedia/src/main/java/org/wikipedia/page/snippet/NoTextSelectedShareAdapter.java
index 1919724..3c97c13 100644
---
a/wikipedia/src/main/java/org/wikipedia/page/snippet/NoTextSelectedShareAdapter.java
+++
b/wikipedia/src/main/java/org/wikipedia/page/snippet/NoTextSelectedShareAdapter.java
@@ -12,21 +12,18 @@
/**
* Share the first paragraph text since no text was selected.
*/
-public class NoTextSelectedShareAdapter {
- private final PageActivity activity;
+public class NoTextSelectedShareAdapter extends ShareHandler {
public NoTextSelectedShareAdapter(PageActivity activity) {
- this.activity = activity;
+ super(activity);
}
public void share() {
- final PageViewFragmentInternal curPageFragment =
activity.getCurPageFragment();
+ final PageViewFragmentInternal curPageFragment =
getActivity().getCurPageFragment();
if (curPageFragment == null) {
return;
}
-
- new ShareHandler(activity, /* TODO */ null)
- .shareSnippet(getFirstParagraphText(curPageFragment), true);
+ shareSnippet(getFirstParagraphText(curPageFragment), true);
}
private CharSequence getFirstParagraphText(final PageViewFragmentInternal
curPageFragment) {
diff --git
a/wikipedia/src/main/java/org/wikipedia/page/snippet/ShareHandler.java
b/wikipedia/src/main/java/org/wikipedia/page/snippet/ShareHandler.java
index bed9447..e96f519 100644
--- a/wikipedia/src/main/java/org/wikipedia/page/snippet/ShareHandler.java
+++ b/wikipedia/src/main/java/org/wikipedia/page/snippet/ShareHandler.java
@@ -1,5 +1,6 @@
package org.wikipedia.page.snippet;
+import android.app.Dialog;
import android.content.DialogInterface;
import android.graphics.Bitmap;
import android.view.View;
@@ -13,15 +14,34 @@
import org.wikipedia.util.ShareUtils;
/**
- * Let user chose between sharing as text or as image.
+ * Let user choose between sharing as text or as image.
*/
public class ShareHandler {
private final PageActivity activity;
- private final ShareAFactFunnel funnel;
+ private Dialog shareDialog;
+ private ShareAFactFunnel funnel;
- public ShareHandler(PageActivity activity, ShareAFactFunnel funnel) {
- this.activity = activity;
+ protected PageActivity getActivity() {
+ return activity;
+ }
+
+ protected ShareAFactFunnel getFunnel() {
+ return funnel;
+ }
+
+ protected void setFunnel(ShareAFactFunnel funnel) {
this.funnel = funnel;
+ }
+
+ public ShareHandler(PageActivity activity) {
+ this.activity = activity;
+ }
+
+ public void onStop() {
+ if (shareDialog != null) {
+ shareDialog.dismiss();
+ shareDialog = null;
+ }
}
public void shareSnippet(CharSequence input, boolean preferUrl) {
@@ -43,20 +63,19 @@
title.getDisplayText(),
curPageFragment.getPage().getPageProperties().isMainPage()
? "" : title.getDescription(),
selectedText).createImage();
- if (preferUrl) {
- new PreviewDialog(activity, resultBitmap,
title.getDisplayText(), introText,
- selectedText, title.getCanonicalUri(), funnel).show();
- } else {
- new PreviewDialog(activity, resultBitmap,
title.getDisplayText(), introText,
- selectedText, selectedText, funnel).show();
+ if (shareDialog != null) {
+ shareDialog.dismiss();
}
+ shareDialog = new PreviewDialog(activity, resultBitmap,
title.getDisplayText(), introText,
+ selectedText, preferUrl ? title.getCanonicalUri() :
selectedText, funnel);
+ shareDialog.show();
} else {
// only share the URL
ShareUtils.shareText(activity, title.getDisplayText(),
title.getCanonicalUri());
}
}
- private String sanitizeText(String selectedText) {
+ private static String sanitizeText(String selectedText) {
return selectedText.replaceAll("\\[\\d+\\]", "") // [1]
.replaceAll("\\(\\s*;\\s*", "\\(") // (; -> ( hacky way for
IPA remnants
.replaceAll("\\s{2,}", " ")
diff --git
a/wikipedia/src/main/java/org/wikipedia/page/snippet/TextSelectedShareAdapter.java
b/wikipedia/src/main/java/org/wikipedia/page/snippet/TextSelectedShareAdapter.java
index 1ae68b3..927966b 100644
---
a/wikipedia/src/main/java/org/wikipedia/page/snippet/TextSelectedShareAdapter.java
+++
b/wikipedia/src/main/java/org/wikipedia/page/snippet/TextSelectedShareAdapter.java
@@ -15,24 +15,47 @@
/**
* Allows sharing selected text in the WebView.
*/
-public class TextSelectedShareAdapter {
- private final PageActivity activity;
+public class TextSelectedShareAdapter extends ShareHandler {
private final WikipediaApp app;
private ActionMode webViewActionMode;
- private static ClipboardManager.OnPrimaryClipChangedListener CLIP_LISTENER;
+ private ClipboardManager clipboard;
+ private ClipboardManager.OnPrimaryClipChangedListener clipChangedListener;
private MenuItem copyMenuItem;
private MenuItem shareItem;
- private ShareAFactFunnel funnel;
+ private boolean expectClipChange = false;
- public TextSelectedShareAdapter(PageActivity activity) {
- this.activity = activity;
- app = (WikipediaApp) activity.getApplicationContext();
+ @TargetApi(11)
+ public TextSelectedShareAdapter(PageActivity parentActivity) {
+ super(parentActivity);
+ app = (WikipediaApp) parentActivity.getApplicationContext();
+ clipboard = (ClipboardManager)
parentActivity.getSystemService(Context.CLIPBOARD_SERVICE);
+ // add our clipboard listener, so that we'll get an event when the text
+ // is copied onto it...
+ clipChangedListener = new
ClipboardManager.OnPrimaryClipChangedListener() {
+ @Override
+ public void onPrimaryClipChanged() {
+ // get the text from the clipboard!
+ if (expectClipChange && clipboard.hasPrimaryClip()
+ && clipboard.getPrimaryClip().getItemCount() > 0) {
+
+ expectClipChange = false;
+ CharSequence selectedText =
clipboard.getPrimaryClip().getItemAt(0)
+
.coerceToText(getActivity());
+ Log.d("Share", ">>> Clipboard text: " + selectedText);
+
+ // Pass the clipboard text to a new Share handler!
+ shareSnippet(selectedText, false);
+ }
+ }
+ };
+ clipboard.addPrimaryClipChangedListener(clipChangedListener);
}
- public void finish() {
- if (webViewActionMode != null) {
- webViewActionMode.finish();
- }
+ @Override
+ @TargetApi(11)
+ public void onStop() {
+ clipboard.removePrimaryClipChangedListener(clipChangedListener);
+ super.onStop();
}
/**
@@ -41,7 +64,6 @@
* method as TargetApi(11), so that the IDE doesn't get upset.
* @param mode ActionMode under which this context is starting.
*/
- @TargetApi(11)
public void onTextSelected(final ActionMode mode) {
webViewActionMode = mode;
Menu menu = mode.getMenu();
@@ -52,7 +74,7 @@
// consistent throughout the various APIs.
for (int i = 0; i < menu.size(); i++) {
String resourceName
- =
activity.getResources().getResourceName(menu.getItem(i).getItemId());
+ =
getActivity().getResources().getResourceName(menu.getItem(i).getItemId());
if (resourceName.contains("action_menu_copy")) {
copyMenuItem = menu.getItem(i);
break;
@@ -65,43 +87,12 @@
// consistent throughout the various APIs.
for (int i = 0; i < menu.size(); i++) {
String resourceName
- =
activity.getResources().getResourceName(menu.getItem(i).getItemId());
+ =
getActivity().getResources().getResourceName(menu.getItem(i).getItemId());
if (resourceName.contains("action_menu_share")) {
shareItem = menu.getItem(i);
break;
}
}
-
- // add our clipboard listener, so that we'll get an event when the text
- // is copied onto it...
- ClipboardManager clipboard = (ClipboardManager)
activity.getSystemService(
- Context.CLIPBOARD_SERVICE);
- if (CLIP_LISTENER == null) {
- CLIP_LISTENER = new
ClipboardManager.OnPrimaryClipChangedListener() {
- @Override
- public void onPrimaryClipChanged() {
- // get the text from the clipboard!
- ClipboardManager clipboard = (ClipboardManager)
activity.getSystemService(
- Context.CLIPBOARD_SERVICE);
- if (clipboard.hasPrimaryClip()
- && clipboard.getPrimaryClip().getItemCount() > 0) {
-
- CharSequence selectedText =
clipboard.getPrimaryClip().getItemAt(0)
- .coerceToText(activity);
- Log.d("Share", ">>> Clipboard text: " + selectedText);
-
- // Pass the clipboard text to a new Share handler!
- new ShareHandler(activity,
funnel).shareSnippet(selectedText, false);
- }
- clipboard.removePrimaryClipChangedListener(CLIP_LISTENER);
- }
- };
- }
- // remove it first, just in case it was added from the last context,
and
- // ended up not being used.
- clipboard.removePrimaryClipChangedListener(CLIP_LISTENER);
- // and add it again.
- clipboard.addPrimaryClipChangedListener(CLIP_LISTENER);
// intercept share menu...
shareItem.setOnMenuItemClickListener(new
MenuItem.OnMenuItemClickListener() {
@@ -109,6 +100,7 @@
public boolean onMenuItemClick(MenuItem item) {
if (copyMenuItem != null) {
// programmatically invoke the copy-to-clipboard action...
+ expectClipChange = true;
webViewActionMode.getMenu()
.performIdentifierAction(copyMenuItem.getItemId(),
0);
// this will trigger a state-change event in the
Clipboard, which we'll
@@ -117,15 +109,16 @@
// leave context mode...
if (webViewActionMode != null) {
webViewActionMode.finish();
+ webViewActionMode = null;
}
return true;
}
});
- final Page page = activity.getCurPageFragment().getPage();
+ final Page page = getActivity().getCurPageFragment().getPage();
final PageProperties pageProperties = page.getPageProperties();
- funnel = new ShareAFactFunnel(app, page.getTitle(),
pageProperties.getPageId(),
- pageProperties.getRevisionId());
- funnel.logHighlight();
+ setFunnel(new ShareAFactFunnel(app, page.getTitle(),
pageProperties.getPageId(),
+ pageProperties.getRevisionId()));
+ getFunnel().logHighlight();
}
}
--
To view, visit https://gerrit.wikimedia.org/r/191775
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib081a48ef342c0844fa996c94314d41b6ff9d298
Gerrit-PatchSet: 4
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <[email protected]>
Gerrit-Reviewer: BearND <[email protected]>
Gerrit-Reviewer: Brion VIBBER <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
