Bug 923391 - Prompts should return values even if they're cancelled. r=mfinkle
authorWes Johnston <wjohnston@mozilla.com>
Tue, 15 Oct 2013 00:13:13 -0700
changeset 165598 7b367ca427b63d7530dd69c62112dbee2dc7cae1
parent 165597 c5cc4233220fc27b0660fdf3c125482be0f06291
child 165599 344307c027959e537e5d669f1d85f33a57097135
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmfinkle
bugs923391
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 923391 - Prompts should return values even if they're cancelled. r=mfinkle
mobile/android/base/prompts/Prompt.java
mobile/android/components/PromptService.js
--- a/mobile/android/base/prompts/Prompt.java
+++ b/mobile/android/base/prompts/Prompt.java
@@ -159,53 +159,82 @@ public class Prompt implements OnClickLi
     public void setButtons(String[] buttons) {
         mButtons = buttons;
     }
 
     public void setInputs(PromptInput[] inputs) {
         mInputs = inputs;
     }
 
+    /* Adds to a result value from the lists that can be shown in dialogs.
+     *  Will set the selected value(s) to the button attribute of the
+     *  object that's passed in. If this is a multi-select dialog, can set
+     *  the button attribute to an array.
+     */
+    private void addListResult(final JSONObject result, int which) {
+        try {
+            if (mSelected != null) {
+                JSONArray selected = new JSONArray();
+                for (int i = 0; i < mSelected.length; i++) {
+                    selected.put(mSelected[i]);
+                }
+                result.put("button", selected);
+            } else {
+                result.put("button", which);
+            }
+        } catch(JSONException ex) { }
+    }
+
+    /* Adds to a result value from the inputs that can be shown in dialogs.
+     * Each input will set its own value in the result.
+     */
+    private void addInputValues(final JSONObject result) {
+        try {
+            if (mInputs != null) {
+                for (int i = 0; i < mInputs.length; i++) {
+                    result.put(mInputs[i].getId(), mInputs[i].getValue());
+                }
+            }
+        } catch(JSONException ex) { }
+    }
+
+    /* Adds the selected button to a result. This should only be called if there
+     * are no lists shown on the dialog, since they also write their results to the button
+     * attribute.
+     */
+    private void addButtonResult(final JSONObject result, int which) {
+        int button = -1;
+        switch(which) {
+            case DialogInterface.BUTTON_POSITIVE : button = 0; break;
+            case DialogInterface.BUTTON_NEUTRAL  : button = 1; break;
+            case DialogInterface.BUTTON_NEGATIVE : button = 2; break;
+        }
+        try {
+            result.put("button", button);
+        } catch(JSONException ex) { }
+    }
+
     @Override
-    public void onClick(DialogInterface aDialog, int aWhich) {
+    public void onClick(DialogInterface dialog, int which) {
         ThreadUtils.assertOnUiThread();
         JSONObject ret = new JSONObject();
         try {
-            int button = -1;
             ListView list = mDialog.getListView();
             if (list != null || mSelected != null) {
-                button = aWhich;
-                if (mSelected != null) {
-                    JSONArray selected = new JSONArray();
-                    for (int i = 0; i < mSelected.length; i++) {
-                        selected.put(mSelected[i]);
-                    }
-                    ret.put("button", selected);
-                } else {
-                    ret.put("button", button);
-                }
+                addListResult(ret, which);
             } else {
-                switch(aWhich) {
-                    case DialogInterface.BUTTON_POSITIVE : button = 0; break;
-                    case DialogInterface.BUTTON_NEUTRAL  : button = 1; break;
-                    case DialogInterface.BUTTON_NEGATIVE : button = 2; break;
-                }
-                ret.put("button", button);
+                addButtonResult(ret, which);
             }
-            if (mInputs != null) {
-                for (int i = 0; i < mInputs.length; i++) {
-                    ret.put(mInputs[i].getId(), mInputs[i].getValue());
-                }
-            }
+            addInputValues(ret);
         } catch(Exception ex) {
             Log.i(LOGTAG, "Error building return: " + ex);
         }
 
-        if (mDialog != null) {
-            mDialog.dismiss();
+        if (dialog != null) {
+            dialog.dismiss();
         }
 
         finishDialog(ret);
     }
 
     /* Adds a set of list items to the prompt. This can be used for either context menu type dialogs, checked lists,
      * or multiple selection lists. If mSelected is set in the prompt before addlistItems is called, the items will be
      * shown with "checkmarks" on their left side.
@@ -313,42 +342,60 @@ public class Prompt implements OnClickLi
                 ScrollView view = new ScrollView(mContext);
                 view.addView(linearLayout);
                 builder.setView(applyInputStyle(view));
             }
         } catch(Exception ex) {
             Log.e(LOGTAG, "Error showing prompt inputs", ex);
             // We cannot display these input widgets with this sdk version,
             // do not display any dialog and finish the prompt now.
-            try {
-                finishDialog(new JSONObject("{\"button\": -1}"));
-            } catch(JSONException e) { }
-
+            cancelDialog();
             return false;
         }
 
         return true;
     }
 
+    /* AdapterView.OnItemClickListener
+     * Called when a list item is clicked
+     */
     @Override
     public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
         ThreadUtils.assertOnUiThread();
         mSelected[position] = !mSelected[position];
     }
 
+    /* @DialogInterface.OnCancelListener
+     * Called when the user hits back to cancel a dialog. The dialog will close itself when this
+     * ends. Setup the correct return values here.
+     *
+     * @param aDialog
+     *          A dialog interface for the dialog that's being closed.
+     */
     @Override
     public void onCancel(DialogInterface aDialog) {
         ThreadUtils.assertOnUiThread();
+        cancelDialog();
+    }
+
+    /* Called in situations where we want to cancel the dialog . This can happen if the user hits back,
+     *  or if the dialog can't be created because of invalid JSON.
+     */
+    private void cancelDialog() {
         JSONObject ret = new JSONObject();
         try {
             ret.put("button", -1);
         } catch(Exception ex) { }
+        addInputValues(ret);
         finishDialog(ret);
     }
 
+    /* Called any time we're closing the dialog to cleanup and notify listeners that the dialog
+     * is closing.
+     */
     public void finishDialog(JSONObject aReturn) {
         mInputs = null;
         mButtons = null;
         mDialog = null;
         mSelected = null;
         try {
             aReturn.put("guid", mGuid);
         } catch(JSONException ex) { }
@@ -361,20 +408,22 @@ public class Prompt implements OnClickLi
         GeckoAppShell.sendEventToGecko(GeckoEvent.createNoOpEvent());
 
         if (mCallback != null) {
             mCallback.onPromptFinished(aReturn.toString());
         }
         mGuid = null;
     }
 
+    /* Handles parsing the initial JSON sent to show dialogs
+     */
     private void processMessage(JSONObject geckoObject) {
-        String title = getSafeString(geckoObject, "title");
-        String text = getSafeString(geckoObject, "text");
-        mGuid = getSafeString(geckoObject, "guid");
+        String title = geckoObject.optString("title");
+        String text = geckoObject.optString("text");
+        mGuid = geckoObject.optString("guid");
 
         mButtons = getStringArray(geckoObject, "buttons");
 
         JSONArray inputs = getSafeArray(geckoObject, "inputs");
         mInputs = new PromptInput[inputs.length()];
         for (int i = 0; i < mInputs.length; i++) {
             try {
                 mInputs[i] = PromptInput.getInput(inputs.getJSONObject(i));
@@ -382,48 +431,24 @@ public class Prompt implements OnClickLi
         }
 
         PromptListItem[] menuitems = getListItemArray(geckoObject, "listitems");
         mSelected = getBooleanArray(geckoObject, "selected");
         boolean multiple = geckoObject.optBoolean("multiple");
         show(title, text, menuitems, multiple);
     }
 
-    private static String getSafeString(JSONObject json, String key) {
-        try {
-            return json.getString(key);
-        } catch (Exception e) {
-            return "";
-        }
-    }
-
     private static JSONArray getSafeArray(JSONObject json, String key) {
         try {
             return json.getJSONArray(key);
         } catch (Exception e) {
             return new JSONArray();
         }
     }
 
-    private static boolean getSafeBool(JSONObject json, String key) {
-        try {
-            return json.getBoolean(key);
-        } catch (Exception e) {
-            return false;
-        }
-    }
-
-    private static int getSafeInt(JSONObject json, String key ) {
-        try {
-            return json.getInt(key);
-        } catch (Exception e) {
-            return 0;
-        }
-    }
-
     public static String[] getStringArray(JSONObject aObject, String aName) {
         JSONArray items = getSafeArray(aObject, aName);
         int length = items.length();
         String[] list = new String[length];
         for (int i = 0; i < length; i++) {
             try {
                 list[i] = items.getString(i);
             } catch(Exception ex) { }
@@ -465,22 +490,22 @@ public class Prompt implements OnClickLi
         public final boolean disabled;
         public final int id;
         public final boolean isParent;
 
         // This member can't be accessible from JS, see bug 733749.
         public Drawable icon;
 
         PromptListItem(JSONObject aObject) {
-            label = getSafeString(aObject, "label");
-            isGroup = getSafeBool(aObject, "isGroup");
-            inGroup = getSafeBool(aObject, "inGroup");
-            disabled = getSafeBool(aObject, "disabled");
-            id = getSafeInt(aObject, "id");
-            isParent = getSafeBool(aObject, "isParent");
+            label = aObject.optString("label");
+            isGroup = aObject.optBoolean("isGroup");
+            inGroup = aObject.optBoolean("inGroup");
+            disabled = aObject.optBoolean("disabled");
+            id = aObject.optInt("id");
+            isParent = aObject.optBoolean("isParent");
         }
 
         public PromptListItem(String aLabel) {
             label = aLabel;
             isGroup = false;
             inGroup = false;
             disabled = false;
             id = 0;
--- a/mobile/android/components/PromptService.js
+++ b/mobile/android/components/PromptService.js
@@ -219,31 +219,31 @@ InternalPrompt.prototype = {
   alert: function alert(aTitle, aText) {
     let p = this._getPrompt(aTitle, aText, [ PromptUtils.getLocaleString("OK") ]);
     this.showPrompt(p);
   },
 
   alertCheck: function alertCheck(aTitle, aText, aCheckMsg, aCheckState) {
     let p = this._getPrompt(aTitle, aText, [ PromptUtils.getLocaleString("OK") ], aCheckMsg, aCheckState);
     let data = this.showPrompt(p);
-    if (aCheckState)
+    if (aCheckState && data.button > -1)
       aCheckState.value = data.checkbox0 == "true";
   },
 
   confirm: function confirm(aTitle, aText) {
     let p = this._getPrompt(aTitle, aText);
     let data = this.showPrompt(p);
     return (data.button == 0);
   },
 
   confirmCheck: function confirmCheck(aTitle, aText, aCheckMsg, aCheckState) {
     let p = this._getPrompt(aTitle, aText, null, aCheckMsg, aCheckState);
     let data = this.showPrompt(p);
     let ok = data.button == 0;
-    if (aCheckState)
+    if (aCheckState && data.button > -1)
       aCheckState.value = data.checkbox0 == "true";
     return ok;
   },
 
   confirmEx: function confirmEx(aTitle, aText, aButtonFlags, aButton0,
                       aButton1, aButton2, aCheckMsg, aCheckState) {
     let buttons = [];
     let titles = [aButton0, aButton1, aButton2];
@@ -279,31 +279,31 @@ InternalPrompt.prototype = {
       if (bTitle)
         buttons.push(bTitle);
 
       aButtonFlags >>= 8;
     }
 
     let p = this._getPrompt(aTitle, aText, buttons, aCheckMsg, aCheckState);
     let data = this.showPrompt(p);
-    if (aCheckState)
+    if (aCheckState && data.button > -1)
       aCheckState.value = data.checkbox0 == "true";
     return data.button;
   },
 
   nsIPrompt_prompt: function nsIPrompt_prompt(aTitle, aText, aValue, aCheckMsg, aCheckState) {
     let p = this._getPrompt(aTitle, aText, null, aCheckMsg, aCheckState);
     p.addTextbox({
       value: aValue.value,
       autofocus: true
     });
     let data = this.showPrompt(p);
 
     let ok = data.button == 0;
-    if (aCheckState)
+    if (aCheckState && data.button > -1)
       aCheckState.value = data.checkbox0 == "true";
     if (ok)
       aValue.value = data.textbox0;
     return ok;
   },
 
   nsIPrompt_promptPassword: function nsIPrompt_promptPassword(
       aTitle, aText, aPassword, aCheckMsg, aCheckState) {
@@ -311,17 +311,17 @@ InternalPrompt.prototype = {
     p.addPassword({
       value: aPassword.value || "",
       autofocus: true,
       hint: PromptUtils.getLocaleString("password", "passwdmgr")
     });
     let data = this.showPrompt(p);
 
     let ok = data.button == 0;
-    if (aCheckState)
+    if (aCheckState && data.button > -1)
       aCheckState.value = data.checkbox0 == "true";
     if (ok)
       aPassword.value = data.password0;
     return ok;
   },
 
   nsIPrompt_promptUsernameAndPassword: function nsIPrompt_promptUsernameAndPassword(
       aTitle, aText, aUsername, aPassword, aCheckMsg, aCheckState) {
@@ -332,17 +332,17 @@ InternalPrompt.prototype = {
       hint: PromptUtils.getLocaleString("username", "passwdmgr")
     }).addPassword({
       value: aPassword.value,
       hint: PromptUtils.getLocaleString("password", "passwdmgr")
     });
     let data = this.showPrompt(p);
 
     let ok = data.button == 0;
-    if (aCheckState)
+    if (aCheckState && data.button > -1)
       aCheckState.value = data.checkbox0 == "true";
     if (ok) {
       aUsername.value = data.textbox0;
       aPassword.value = data.password0;
     }
     return ok;
   },