Fixed: EHH - Filter creating window won't close

Fixed: EHH - Filter creating window won't close

[Wladimir Palant, 2012-11-08 10:42:33]

http://codereview.adblockplus.org/8788069/diff/1/chrome/content/composer.js
File chrome/content/composer.js (right):

http://codereview.adblockplus.org/8788069/diff/1/chrome/content/composer.js#n...
chrome/content/composer.js:138: wnd.addEventListener("unload", function(event){
togglePreview(false); }, true);
So togglePreview() is the issue? Then this is the wrong solution.

1) We are cleaning up when the window unloads - that's pointless, nobody will
see that window.
2) Adding an "unload" event listener has the side-effect of disabling fast
back&forward for that page.

How about checking wnd.closed in togglePreview()?

[Andrey Novikov, 2012-11-08 10:54:54]

http://codereview.adblockplus.org/8788069/diff/1/chrome/content/composer.js
File chrome/content/composer.js (right):

http://codereview.adblockplus.org/8788069/diff/1/chrome/content/composer.js#n...
chrome/content/composer.js:138: wnd.addEventListener("unload", function(event){
togglePreview(false); }, true);
Exact issue is that previewStyle is dead object after reload. I can not check
wnd.closed because it's not closed, it was jast reloaded. Any other way to hook
reload event? I've tried document.unload but it didn't fire.

[Wladimir Palant, 2012-11-08 14:19:49]

On 2012/11/08 10:54:54, Andrey Novikov wrote:
> I can not check
> wnd.closed because it's not closed, it was jast reloaded. Any other way to
hook
> reload event? I've tried document.unload but it didn't fire.

The other way would be to use the pagehide event - but it doesn't guarantee that
the page is gone, the user might come back to that page. Sounds like something
you would use weak references for but in this case it should be simpler to catch
the exception from accessing previewStyle.parentNode. Please add a link to
https://bugzilla.mozilla.org/show_bug.cgi?id=695480 to explain why that
exception occurs.

[Wladimir Palant, 2012-11-09 12:12:14]

LGTM