Last Comment Bug 706746 - notification bar button handler should indicate it has closed the notification bar
: notification bar button handler should indicate it has closed the notificatio...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 11
Assigned To: Cameron McCormack (:heycam) (away Jun 25 – Jul 10)
:
Mentors:
Depends on:
Blocks: 702050
  Show dependency treegraph
 
Reported: 2011-11-30 20:56 PST by Cameron McCormack (:heycam) (away Jun 25 – Jul 10)
Modified: 2011-12-02 03:24 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Don't try to close the Inspector's "Leaving this page" notification bar twice. (1.16 KB, patch)
2011-11-30 20:58 PST, Cameron McCormack (:heycam) (away Jun 25 – Jul 10)
mihai.sucan: review+
Details | Diff | Review

Description Cameron McCormack (:heycam) (away Jun 25 – Jul 10) 2011-11-30 20:56:21 PST
The handler for the button on the "Leaving this page will close the Inspector and the changes you have made will be lost." notification bar needs to return true if it causes the notification bar to disappear, which it does by calling closeInspectorUI().
Comment 1 Cameron McCormack (:heycam) (away Jun 25 – Jul 10) 2011-11-30 20:58:15 PST
Created attachment 578171 [details] [diff] [review]
Don't try to close the Inspector's "Leaving this page" notification bar twice.
Comment 2 Mihai Sucan [:msucan] 2011-12-01 12:12:31 PST
Comment on attachment 578171 [details] [diff] [review]
Don't try to close the Inspector's "Leaving this page" notification bar twice.

Review of attachment 578171 [details] [diff] [review]:
-----------------------------------------------------------------

This is a slip of mine back when I wrote the code. Thanks for your fix!
Comment 3 Dão Gottwald [:dao] 2011-12-01 13:06:43 PST
Comment on attachment 578171 [details] [diff] [review]
Don't try to close the Inspector's "Leaving this page" notification bar twice.

>         callback: function onButtonLeave() {
>           if (aRequest) {
>             aRequest.resume();
>             aRequest = null;
>             this.IUI.closeInspectorUI();
>+            return true;
>           }
>         }.bind(this),

You probably want "return false;" at the end, to prevent the "function onButtonLeave does not always return a value" warning.
Comment 4 Cameron McCormack (:heycam) (away Jun 25 – Jul 10) 2011-12-01 16:00:22 PST
I don't get that warning when running the test, but I will include it for clarity.
Comment 5 Cameron McCormack (:heycam) (away Jun 25 – Jul 10) 2011-12-01 16:15:43 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab56cc75af51
Comment 6 Dão Gottwald [:dao] 2011-12-01 20:53:14 PST
(In reply to Cameron McCormack (:heycam) from comment #4)
> I don't get that warning when running the test

You need to enable javascript.options.strict.
Comment 7 Marco Bonardo [::mak] 2011-12-02 03:24:51 PST
https://hg.mozilla.org/mozilla-central/rev/ab56cc75af51

Note You need to log in before you can comment on or make changes to this bug.