Bug 884069 - Refactor DoorHanger to create a better Java API that doesn't depend on JSON. r=sriram,wesj
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Fri, 21 Jun 2013 20:36:35 -0700
changeset 136073 4f98cdea7ea41b8295df48a0ea68041dbc5c2e2f
parent 136072 0a7b5696c49afa31bb6ff9aed758a6512a7c1f39
child 136074 e9b1e686b98389ccd7e5e08da025efd76b49d8c3
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewerssriram, wesj
bugs884069
milestone24.0a1
Bug 884069 - Refactor DoorHanger to create a better Java API that doesn't depend on JSON. r=sriram,wesj
mobile/android/base/DoorHanger.java
mobile/android/base/DoorHangerPopup.java
--- a/mobile/android/base/DoorHanger.java
+++ b/mobile/android/base/DoorHanger.java
@@ -10,16 +10,17 @@ import org.mozilla.gecko.widget.Divider;
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 
 import android.content.Context;
 import android.graphics.Rect;
 import android.os.Build;
 import android.text.SpannableString;
+import android.text.TextUtils;
 import android.text.method.LinkMovementMethod;
 import android.text.style.ForegroundColorSpan;
 import android.text.style.URLSpan;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.Button;
@@ -27,199 +28,174 @@ import android.widget.CheckBox;
 import android.widget.LinearLayout;
 import android.widget.Spinner;
 import android.widget.SpinnerAdapter;
 import android.widget.TextView;
 
 import java.util.ArrayList;
 import java.util.List;
 
-public class DoorHanger extends LinearLayout implements Button.OnClickListener {
+public class DoorHanger extends LinearLayout {
     private static final String LOGTAG = "GeckoDoorHanger";
 
-    // The popup that holds this doorhanger
-    private DoorHangerPopup mPopup;
-    private LinearLayout mChoicesLayout;
-    private TextView mTextView;
-    private List<PromptInput> mInputs;
-
     private static int sInputPadding = -1;
     private static int sSpinnerTextColor = -1;
     private static int sSpinnerTextSize = -1;
 
-    // LayoutParams used for adding button layouts
-    static private LayoutParams mLayoutParams;
+    private static LayoutParams sButtonParams;
+    static {
+        sButtonParams = new LayoutParams(LayoutParams.FILL_PARENT, LayoutParams.FILL_PARENT, 1.0f);
+    }
+
+    private final TextView mTextView;
+    private final LinearLayout mChoicesLayout;
+
+    // Divider between doorhangers.
+    private final View mDivider;
+
+    // The tab associated with this notification.
     private final int mTabId;
-    // Value used to identify the notification
+
+    // Value used to identify the notification.
     private final String mValue;
 
-    // Optional checkbox added underneath message text
+    private List<PromptInput> mInputs;
     private CheckBox mCheckBox;
 
-    // Divider between doorhangers.
-    private View mDivider;
-
     private int mPersistence = 0;
     private boolean mPersistWhileVisible = false;
     private long mTimeout = 0;
 
-    DoorHanger(Context context, DoorHangerPopup popup, int tabId, String value) {
+    public interface OnButtonClickListener {
+        public void onButtonClick(DoorHanger dh, String tag);
+    }
+
+    DoorHanger(Context context) {
+        this(context, 0, null);
+    }
+
+    DoorHanger(Context context, int tabId, String value) {
         super(context);
 
-        mPopup = popup;
         mTabId = tabId;
         mValue = value;
 
         if (sInputPadding == -1) {
             sInputPadding = getResources().getDimensionPixelSize(R.dimen.doorhanger_padding);
         }
         if (sSpinnerTextColor == -1) {
             sSpinnerTextColor = getResources().getColor(R.color.text_color_primary_disable_only);
         }
         if (sSpinnerTextSize == -1) {
             sSpinnerTextSize = getResources().getDimensionPixelSize(R.dimen.doorhanger_spinner_textsize);
         }
+
+        setOrientation(VERTICAL);
+
+        LayoutInflater.from(context).inflate(R.layout.doorhanger, this);
+        mTextView = (TextView) findViewById(R.id.doorhanger_title);
+        mChoicesLayout = (LinearLayout) findViewById(R.id.doorhanger_choices);
+        mDivider = findViewById(R.id.divider_doorhanger);
     }
- 
+
     int getTabId() {
         return mTabId;
     }
 
     String getValue() {
         return mValue;
     }
 
-    public void showDivider() {
+    List<PromptInput> getInputs() {
+        return mInputs;
+    }
+
+    CheckBox getCheckBox() {
+        return mCheckBox;
+    }
+
+    void showDivider() {
         mDivider.setVisibility(View.VISIBLE);
     }
 
-    public void hideDivider() {
+    void hideDivider() {
         mDivider.setVisibility(View.GONE);
     }
 
-    // Postpone stuff that needs to be done on the main thread
-    void init(String message, JSONArray buttons, JSONObject options) {
-        setOrientation(VERTICAL);
-
-        LayoutInflater.from(getContext()).inflate(R.layout.doorhanger, this);
-        setVisibility(View.GONE);
-
-        mTextView = (TextView) findViewById(R.id.doorhanger_title);
+    void setMessage(String message) {
         mTextView.setText(message);
-
-        mChoicesLayout = (LinearLayout) findViewById(R.id.doorhanger_choices);
-
-        mDivider = findViewById(R.id.divider_doorhanger);
-
-        // Set the doorhanger text and buttons
-        for (int i = 0; i < buttons.length(); i++) {
-            try {
-                JSONObject buttonObject = buttons.getJSONObject(i);
-                String label = buttonObject.getString("label");
-                int callBackId = buttonObject.getInt("callback");
-                addButton(label, callBackId);
-            } catch (JSONException e) {
-                Log.e(LOGTAG, "Error creating doorhanger button", e);
-            }
-         }
-
-        // Enable the button layout if we have buttons.
-        if (buttons.length() > 0) {
-            findViewById(R.id.divider_choices).setVisibility(View.VISIBLE);
-            mChoicesLayout.setVisibility(View.VISIBLE);
-        }
-
-        setOptions(options);
     }
 
-    private void addButton(String aText, int aCallback) {
-        if (mLayoutParams == null)
-            mLayoutParams = new LayoutParams(LayoutParams.FILL_PARENT,
-                                             LayoutParams.FILL_PARENT,
-                                             1.0f);
+    void addLink(String label, String url, String delimiter) {
+        String title = mTextView.getText().toString();
+        SpannableString titleWithLink = new SpannableString(title + delimiter + label);
+        URLSpan linkSpan = new URLSpan(url) {
+            @Override
+            public void onClick(View view) {
+                Tabs.getInstance().loadUrlInTab(getURL());
+            }
+        };
+
+        // Prevent text outside the link from flashing when clicked.
+        ForegroundColorSpan colorSpan = new ForegroundColorSpan(mTextView.getCurrentTextColor());
+        titleWithLink.setSpan(colorSpan, 0, title.length(), 0);
+
+        titleWithLink.setSpan(linkSpan, title.length() + 1, titleWithLink.length(), 0);
+        mTextView.setText(titleWithLink);
+        mTextView.setMovementMethod(LinkMovementMethod.getInstance());
+    }
 
-        Button button = (Button) LayoutInflater.from(getContext()).inflate(R.layout.doorhanger_button, null);
-        button.setText(aText);
-        button.setTag(Integer.toString(aCallback));
-        button.setOnClickListener(this);
+    void addButton(final String text, final String tag, final OnButtonClickListener listener) {
+        final Button button = (Button) LayoutInflater.from(getContext()).inflate(R.layout.doorhanger_button, null);
+        button.setText(text);
+        button.setTag(tag);
 
-        if (mChoicesLayout.getChildCount() > 0) {
+        button.setOnClickListener(new Button.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                listener.onButtonClick(DoorHanger.this, tag);
+            }
+        });
+
+        if (mChoicesLayout.getChildCount() == 0) {
+            // If this is the first button we're adding, make the choices layout visible.
+            mChoicesLayout.setVisibility(View.VISIBLE);
+            findViewById(R.id.divider_choices).setVisibility(View.VISIBLE);
+        } else {
+            // Add a divider for additional buttons.
             Divider divider = new Divider(getContext(), null);
             divider.setOrientation(Divider.Orientation.VERTICAL);
             divider.setBackgroundColor(0xFFD1D5DA);
             mChoicesLayout.addView(divider);
         }
 
-        mChoicesLayout.addView(button, mLayoutParams);
-    }
-
-    @Override
-    public void onClick(View v) {
-        JSONObject response = new JSONObject();
-        try {
-            response.put("callback", v.getTag().toString());
-
-            // If the checkbox is being used, pass its value
-            if (mCheckBox != null)
-                response.put("checked", mCheckBox.isChecked());
-
-            if (mInputs != null) {
-                JSONObject inputs = new JSONObject();
-                for (PromptInput input : mInputs) {
-                    inputs.put(input.getId(), input.getValue());
-                }
-                response.put("inputs", inputs);
-            }
-        } catch (JSONException e) {
-            Log.e(LOGTAG, "Error creating onClick response", e);
-        }
-
-        GeckoEvent e = GeckoEvent.createBroadcastEvent("Doorhanger:Reply", response.toString());
-        GeckoAppShell.sendEventToGecko(e);
-        mPopup.removeDoorHanger(this);
-
-        // This will hide the doorhanger (and hide the popup if there are no
-        // more doorhangers to show)
-        mPopup.updatePopup();
+        mChoicesLayout.addView(button, sButtonParams);
     }
 
-    private void setOptions(final JSONObject options) {
-        try {
-            mPersistence = options.getInt("persistence");
-        } catch (JSONException e) { }
+    void setOptions(final JSONObject options) {
+        final int persistence = options.optInt("persistence");
+        if (persistence > 0) {
+            mPersistence = persistence;
+        }
 
-        try {
-            mPersistWhileVisible = options.getBoolean("persistWhileVisible");
-        } catch (JSONException e) { }
-
-        try {
-            mTimeout = options.getLong("timeout");
-        } catch (JSONException e) { }
+        mPersistWhileVisible = options.optBoolean("persistWhileVisible");
 
-        try {
-            JSONObject link = options.getJSONObject("link");
-            String title = mTextView.getText().toString();
-            String linkLabel = link.getString("label");
-            String linkUrl = link.getString("url");
-            SpannableString titleWithLink = new SpannableString(title + " " + linkLabel);
-            URLSpan linkSpan = new URLSpan(linkUrl) {
-                @Override
-                public void onClick(View view) {
-                    Tabs.getInstance().loadUrlInTab(this.getURL());
-                }
-            };
+        final long timeout = options.optLong("timeout");
+        if (timeout > 0) {
+            mTimeout = timeout;
+        }
 
-            // prevent text outside the link from flashing when clicked
-            ForegroundColorSpan colorSpan = new ForegroundColorSpan(mTextView.getCurrentTextColor());
-            titleWithLink.setSpan(colorSpan, 0, title.length(), 0);
-
-            titleWithLink.setSpan(linkSpan, title.length() + 1, titleWithLink.length(), 0);
-            mTextView.setText(titleWithLink);
-            mTextView.setMovementMethod(LinkMovementMethod.getInstance());
-        } catch (JSONException e) { }
+        final JSONObject link = options.optJSONObject("link");
+        if (link != null) {
+            try {
+                final String linkLabel = link.getString("label");
+                final String linkUrl = link.getString("url");
+                addLink(linkLabel, linkUrl, " ");
+            } catch (JSONException e) { }
+        }
 
         final JSONArray inputs = options.optJSONArray("inputs");
         if (inputs != null) {
             mInputs = new ArrayList<PromptInput>();
 
             final ViewGroup group = (ViewGroup) findViewById(R.id.doorhanger_inputs);
             group.setVisibility(VISIBLE);
 
@@ -230,22 +206,22 @@ public class DoorHanger extends LinearLa
 
                     View v = input.getView(getContext());
                     styleInput(input, v);
                     group.addView(v);
                 } catch(JSONException ex) { }
             }
         }
 
-        try {
-            String checkBoxText = options.getString("checkbox");
+        final String checkBoxText = options.optString("checkbox");
+        if (!TextUtils.isEmpty(checkBoxText)) {
             mCheckBox = (CheckBox) findViewById(R.id.doorhanger_checkbox);
             mCheckBox.setText(checkBoxText);
             mCheckBox.setVisibility(VISIBLE);
-        } catch (JSONException e) { }
+        }
     }
 
     private void styleInput(PromptInput input, View view) {
         if (input instanceof PromptInput.MenulistInput) {
             styleSpinner(input, view);
         } else {
             // add some top and bottom padding to separate inputs
             view.setPadding(0, sInputPadding,
@@ -290,20 +266,25 @@ public class DoorHanger extends LinearLa
             spinInput.textView.setTextColor(sSpinnerTextColor);
             spinInput.textView.setTextSize(sSpinnerTextSize);
 
             // If this spinner has a label, offset it to also be aligned with the doorhanger text.
             spinInput.textView.setPadding(rect.left + textPadding, 0, 0, 0);
         }
     }
 
-    // This method checks with persistence and timeout options to see if
-    // it's okay to remove a doorhanger.
-    boolean shouldRemove() {
-        if (mPersistWhileVisible && mPopup.isShowing()) {
+
+    /*
+     * Checks with persistence and timeout options to see if it's okay to remove a doorhanger.
+     *
+     * @param isShowing Whether or not this doorhanger is currently visible to the user.
+     *                 (e.g. the DoorHanger view might be VISIBLE, but its parent could be hidden)
+     */
+    boolean shouldRemove(boolean isShowing) {
+        if (mPersistWhileVisible && isShowing) {
             // We still want to decrement mPersistence, even if the popup is showing
             if (mPersistence != 0)
                 mPersistence--;
             return false;
         }
 
         // If persistence is set to -1, the doorhanger will never be
         // automatically removed.
--- a/mobile/android/base/DoorHangerPopup.java
+++ b/mobile/android/base/DoorHangerPopup.java
@@ -4,34 +4,39 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
 import org.mozilla.gecko.util.GeckoEventListener;
 import org.mozilla.gecko.widget.ArrowPopup;
 
 import org.json.JSONArray;
+import org.json.JSONException;
 import org.json.JSONObject;
 
 import android.os.Build;
 import android.util.Log;
 import android.view.View;
+import android.widget.CheckBox;
 
 import java.util.HashSet;
+import java.util.List;
 
 public class DoorHangerPopup extends ArrowPopup
-                             implements GeckoEventListener, Tabs.OnTabsChangedListener {
+                             implements GeckoEventListener,
+                                        Tabs.OnTabsChangedListener,
+                                        DoorHanger.OnButtonClickListener {
     private static final String LOGTAG = "GeckoDoorHangerPopup";
 
     // Stores a set of all active DoorHanger notifications. A DoorHanger is
     // uniquely identified by its tabId and value.
     private HashSet<DoorHanger> mDoorHangers;
 
-    DoorHangerPopup(GeckoApp aActivity, View aAnchor) {
-        super(aActivity, aAnchor);
+    DoorHangerPopup(GeckoApp activity, View anchor) {
+        super(activity, anchor);
 
         mDoorHangers = new HashSet<DoorHanger>();
 
         registerEventListener("Doorhanger:Add");
         registerEventListener("Doorhanger:Remove");
         Tabs.registerOnTabsChangedListener(this);
     }
 
@@ -116,38 +121,87 @@ public class DoorHangerPopup extends Arr
     /**
      * Adds a doorhanger.
      *
      * This method must be called on the UI thread.
      */
     void addDoorHanger(final int tabId, final String value, final String message,
                        final JSONArray buttons, final JSONObject options) {
         // Don't add a doorhanger for a tab that doesn't exist
-        if (Tabs.getInstance().getTab(tabId) == null)
+        if (Tabs.getInstance().getTab(tabId) == null) {
             return;
+        }
 
         // Replace the doorhanger if it already exists
         DoorHanger oldDoorHanger = getDoorHanger(tabId, value);
-        if (oldDoorHanger != null)
+        if (oldDoorHanger != null) {
             removeDoorHanger(oldDoorHanger);
+        }
 
-        final DoorHanger newDoorHanger = new DoorHanger(mActivity, this, tabId, value);
+        if (!mInflated) {
+            init();
+        }
+
+        final DoorHanger newDoorHanger = new DoorHanger(mActivity, tabId, value);
+        newDoorHanger.setMessage(message);
+        newDoorHanger.setOptions(options);
+
+        for (int i = 0; i < buttons.length(); i++) {
+            try {
+                JSONObject buttonObject = buttons.getJSONObject(i);
+                String label = buttonObject.getString("label");
+                String tag = String.valueOf(buttonObject.getInt("callback"));
+                newDoorHanger.addButton(label, tag, this);
+            } catch (JSONException e) {
+                Log.e(LOGTAG, "Error creating doorhanger button", e);
+            }
+        }
+
         mDoorHangers.add(newDoorHanger);
-
-        if (!mInflated)
-            init();
-
-        newDoorHanger.init(message, buttons, options);
         mContent.addView(newDoorHanger);
 
         // Only update the popup if we're adding a notifcation to the selected tab
         if (tabId == Tabs.getInstance().getSelectedTab().getId())
             updatePopup();
     }
 
+
+    /*
+     * DoorHanger.OnButtonClickListener implementation
+     */
+    @Override
+    public void onButtonClick(DoorHanger dh, String tag) {
+        JSONObject response = new JSONObject();
+        try {
+            response.put("callback", tag);
+
+            CheckBox checkBox = dh.getCheckBox();
+            // If the checkbox is being used, pass its value
+            if (checkBox != null) {
+                response.put("checked", checkBox.isChecked());
+            }
+
+            List<PromptInput> doorHangerInputs = dh.getInputs();
+            if (doorHangerInputs != null) {
+                JSONObject inputs = new JSONObject();
+                for (PromptInput input : doorHangerInputs) {
+                    inputs.put(input.getId(), input.getValue());
+                }
+                response.put("inputs", inputs);
+            }
+        } catch (JSONException e) {
+            Log.e(LOGTAG, "Error creating onClick response", e);
+        }
+
+        GeckoEvent e = GeckoEvent.createBroadcastEvent("Doorhanger:Reply", response.toString());
+        GeckoAppShell.sendEventToGecko(e);
+        removeDoorHanger(dh);
+        updatePopup();
+    }
+
     /**
      * Gets a doorhanger.
      *
      * This method must be called on the UI thread.
      */
     DoorHanger getDoorHanger(int tabId, String value) {
         for (DoorHanger dh : mDoorHangers) {
             if (dh.getTabId() == tabId && dh.getValue().equals(value))
@@ -173,17 +227,17 @@ public class DoorHangerPopup extends Arr
      *
      * This method must be called on the UI thread.
      */
     void removeTransientDoorHangers(int tabId) {
         // Make a temporary set to avoid a ConcurrentModificationException
         HashSet<DoorHanger> doorHangersToRemove = new HashSet<DoorHanger>();
         for (DoorHanger dh : mDoorHangers) {
             // Only remove transient doorhangers for the given tab
-            if (dh.getTabId() == tabId && dh.shouldRemove())
+            if (dh.getTabId() == tabId && dh.shouldRemove(isShowing()))
                 doorHangersToRemove.add(dh);
         }
 
         for (DoorHanger dh : doorHangersToRemove) {
             removeDoorHanger(dh);
         }
     }