notification bar button handler should indicate it has closed the notification bar

RESOLVED FIXED in Firefox 11

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: heycam, Assigned: heycam)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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().
(Assignee)

Comment 1

6 years ago
Created attachment 578171 [details] [diff] [review]
Don't try to close the Inspector's "Leaving this page" notification bar twice.
(Assignee)

Updated

6 years ago
Attachment #578171 - Flags: review?(mihai.sucan)
(Assignee)

Updated

6 years ago
Blocks: 703176
(Assignee)

Updated

6 years ago
Blocks: 702050
No longer blocks: 703176
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!
Attachment #578171 - Flags: review?(mihai.sucan) → review+
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.
(Assignee)

Comment 4

6 years ago
I don't get that warning when running the test, but I will include it for clarity.
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab56cc75af51
Assignee: nobody → cam
Status: NEW → ASSIGNED
(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.
Status: ASSIGNED → NEW
https://hg.mozilla.org/mozilla-central/rev/ab56cc75af51
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
You need to log in before you can comment on or make changes to this bug.