659591 - Fire SSWindowClosing when session store handles a closing window (on domwindowclosed)

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

From bug 655269 comment #7:

So we should probably do like what we do for tabs (SSTabClosing), which is fired at the beginning of sessionstore processing the TabClose event. We can add SSWindowClosing.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 535038 [details] [diff] [review]
patch v1

Passed try:

http://tbpl.mozilla.org/?tree=Try&pusher=[e-mail]&rev=b11596fb0686

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Comment 4

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 537155 [details] [diff] [review]
patch v2

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

::: browser/components/sessionstore/test/browser/browser_659591.js
@@ +6,5 @@
> +
> +  newWindow(function (win) {
> +    win.addEventListener("SSWindowClosing", function onWindowClosing() {
> +      win.removeEventListener("SSWindowClosing", onWindowClosing, false);
> +      waitForFocus(finish);

Let's make is so there's actually a test that passes. Have a variable (gotEvent or something) defaulted to false. Set to true in the SSWindowClosing handler, then register a cleanup function that checks ok(gotEvent).

That way we'll at least have a passing test and if it fails, it won't just be a timeout, we'll actually see a failed test.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/a1b563094e5a

Comment 6

[Boris Zbarsky [:bzbarsky]]

Backed out due to mochitest-other orange.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/92f4836116a3

Comment 8

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/92f4836116a3