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 27 Jan–1 Feb)
:
: Patrick Brosset <:pbro>
Mentors:
Depends on:
Blocks: 702050
  Show dependency treegraph
 
Reported: 2011-11-30 20:56 PST by Cameron McCormack (:heycam) (away 27 Jan–1 Feb)
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 27 Jan–1 Feb)
mihai.sucan: review+
Details | Diff | Splinter Review

Description User image Cameron McCormack (:heycam) (away 27 Jan–1 Feb) 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 User image Cameron McCormack (:heycam) (away 27 Jan–1 Feb) 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 User image 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 User image 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 User image Cameron McCormack (:heycam) (away 27 Jan–1 Feb) 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 User image Cameron McCormack (:heycam) (away 27 Jan–1 Feb) 2011-12-01 16:15:43 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab56cc75af51
Comment 6 User image 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 User image 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.