829553 - [toolbox] If the last browser tab is closed with ctrl-w, the undocked toolbox is not closed.

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Comment 1

[David Creswick]

Attachment: patch v1

From https://developer.mozilla.org/en-US/docs/Mozilla_event_reference/close_event

> Note that the close event is only fired when the user presses the
> close button on the titlebar; (i.e. not File -> Quit). The unload
> event should be used to capture all attempts to unload the window.

So, the easy fix is for Target to hook into unload events instead of
close events. That works, mostly. Unfortunately, the web console was
written under the assumption that a browser window always exists, so
this patch breaks its destroy method.

> TypeError: this.browserWindow is null: WebConsole.prototype.mainPopupSet@resource:///modules/HUDService.jsm:202
> WC_destroy@resource:///modules/HUDService.jsm:375
> WCP_destroy@resource:///modules/WebConsolePanel.jsm:65
> TBOX_destroy@resource:///modules/devtools/Toolbox.jsm:661
> EventEmitter_once/handler<@resource:///modules/devtools/EventEmitter.jsm:56
> EventEmitter_emit@resource:///modules/devtools/EventEmitter.jsm:100
> TabTarget.prototype.destroy@resource:///modules/devtools/Target.jsm:274
> TabTarget.prototype.handleEvent@resource:///modules/devtools/Target.jsm:233

What is the proper procedure here? Do I open a new bug, fix it there,
and add a dependency from this bug? Do I try to land this patch
immediately, or wait for a web console fix to come along?

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to David Creswick from comment #1)
> What is the proper procedure here? Do I open a new bug, fix it there,
> and add a dependency from this bug? Do I try to land this patch
> immediately, or wait for a web console fix to come along?

Because this patch causes a web console problem then the problem should be fixed as part of this patch. We would normally only open a new bug if the patch worked and something else and was planned that depended on this patch.

Comment 3

[David Creswick]

Attachment: patch v2 - with a fix for web console

This patch fixes the problem by setting WebConsole's "browserWindow" property at object constructor time, where formerly it was set lazily. This guarantees that browserWindow is defined when WebConsole#destroy runs. I ran the full suite of devtools tests locally and didn't find any new failures, so I assume this doesn't affect any other web console functionality.

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 714956 [details] [diff] [review]
patch v2 - with a fix for web console

Simple fix that does the job perfectly.

Great job!

Comment 5

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/9800064fbb22

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/9800064fbb22