Dbrant has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/191775

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 maintain a reference to the Dialog that
they instantiate, 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, 88 insertions(+), 51 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia 
refs/changes/75/191775/1

diff --git a/wikipedia/src/main/java/org/wikipedia/page/PageActivity.java 
b/wikipedia/src/main/java/org/wikipedia/page/PageActivity.java
index dd13998..bc60cec 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,16 @@
 
     @Override
     public void onSupportActionModeFinished(ActionMode mode) {
-        textSelectedShareAdapter = null;
+        if (textSelectedShareAdapter != null) {
+            textSelectedShareAdapter.finish();
+        }
         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..f262f8a 100644
--- 
a/wikipedia/src/main/java/org/wikipedia/page/snippet/NoTextSelectedShareAdapter.java
+++ 
b/wikipedia/src/main/java/org/wikipedia/page/snippet/NoTextSelectedShareAdapter.java
@@ -1,5 +1,6 @@
 package org.wikipedia.page.snippet;
 
+import android.app.Dialog;
 import android.text.Html;
 
 import org.wikipedia.page.PageActivity;
@@ -14,6 +15,7 @@
  */
 public class NoTextSelectedShareAdapter {
     private final PageActivity activity;
+    private Dialog shareDialog;
 
     public NoTextSelectedShareAdapter(PageActivity activity) {
         this.activity = activity;
@@ -25,8 +27,21 @@
             return;
         }
 
-        new ShareHandler(activity, /* TODO */ null)
+        if (shareDialog != null) {
+            shareDialog.dismiss();
+        }
+        shareDialog = new ShareHandler(activity, /* TODO */ null)
                 .shareSnippet(getFirstParagraphText(curPageFragment), true);
+        if (shareDialog != null) {
+            shareDialog.show();
+        }
+    }
+
+    public void onStop() {
+        if (shareDialog != null) {
+            shareDialog.dismiss();
+            shareDialog = null;
+        }
     }
 
     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..f6c7b8e 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;
@@ -24,10 +25,10 @@
         this.funnel = funnel;
     }
 
-    public void shareSnippet(CharSequence input, boolean preferUrl) {
+    public Dialog shareSnippet(CharSequence input, boolean preferUrl) {
         final PageViewFragmentInternal curPageFragment = 
activity.getCurPageFragment();
         if (curPageFragment == null) {
-            return;
+            return null;
         }
 
         String selectedText = sanitizeText(input.toString());
@@ -44,16 +45,17 @@
                     curPageFragment.getPage().getPageProperties().isMainPage() 
? "" : title.getDescription(),
                     selectedText).createImage();
             if (preferUrl) {
-                new PreviewDialog(activity, resultBitmap, 
title.getDisplayText(), introText,
-                        selectedText, title.getCanonicalUri(), funnel).show();
+                return new PreviewDialog(activity, resultBitmap, 
title.getDisplayText(), introText,
+                        selectedText, title.getCanonicalUri(), funnel);
             } else {
-                new PreviewDialog(activity, resultBitmap, 
title.getDisplayText(), introText,
-                        selectedText, selectedText, funnel).show();
+                return new PreviewDialog(activity, resultBitmap, 
title.getDisplayText(), introText,
+                        selectedText, selectedText, funnel);
             }
         } else {
             // only share the URL
             ShareUtils.shareText(activity, title.getDisplayText(), 
title.getCanonicalUri());
         }
+        return null;
     }
 
     private String sanitizeText(String selectedText) {
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..0cad991 100644
--- 
a/wikipedia/src/main/java/org/wikipedia/page/snippet/TextSelectedShareAdapter.java
+++ 
b/wikipedia/src/main/java/org/wikipedia/page/snippet/TextSelectedShareAdapter.java
@@ -1,6 +1,7 @@
 package org.wikipedia.page.snippet;
 
 import android.annotation.TargetApi;
+import android.app.Dialog;
 import android.content.ClipboardManager;
 import android.content.Context;
 import android.support.v7.view.ActionMode;
@@ -19,19 +20,58 @@
     private final PageActivity activity;
     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 Dialog shareDialog;
+    private boolean expectClipChange = false;
 
-    public TextSelectedShareAdapter(PageActivity activity) {
-        this.activity = activity;
+    @TargetApi(11)
+    public TextSelectedShareAdapter(PageActivity parentActivity) {
+        this.activity = parentActivity;
         app = (WikipediaApp) activity.getApplicationContext();
+        clipboard = (ClipboardManager) 
activity.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(activity);
+                    Log.d("Share", ">>> Clipboard text: " + selectedText);
+
+                    if (shareDialog != null) {
+                        shareDialog.dismiss();
+                    }
+                    // Pass the clipboard text to a new Share handler!
+                    shareDialog = new ShareHandler(activity, 
funnel).shareSnippet(selectedText, false);
+                    if (shareDialog != null) {
+                        shareDialog.show();
+                    }
+                }
+            }
+        };
+        clipboard.addPrimaryClipChangedListener(clipChangedListener);
+    }
+
+    @TargetApi(11)
+    public void onStop() {
+        clipboard.removePrimaryClipChangedListener(clipChangedListener);
+        finish();
     }
 
     public void finish() {
-        if (webViewActionMode != null) {
-            webViewActionMode.finish();
+        webViewActionMode = null;
+        if (shareDialog != null) {
+            shareDialog.dismiss();
+            shareDialog = null;
         }
     }
 
@@ -41,7 +81,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();
@@ -72,43 +111,13 @@
             }
         }
 
-        // 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() {
             @Override
             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

-- 
To view, visit https://gerrit.wikimedia.org/r/191775
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib081a48ef342c0844fa996c94314d41b6ff9d298
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to