Avoid accessing dead windows in test-firefox-windows.js

RESOLVED FIXED in Firefox 51

Status

Add-on SDK
General
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: erahm, Assigned: erahm)

Tracking

(Blocks: 1 bug)

unspecified
mozilla51

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment)

This is fallout from bug 1276366 where test-firefox-windows.js ended up accessing dead object wrappers.

Rather than checking for window.closed we can just check the number of active
windows.
Created attachment 8786954 [details] [diff] [review]
Avoid accessing dead windows in test-firefox-windows.js

:zer0 it looks like you did the last review in this area, feel free to redirect.
Attachment #8786954 - Flags: review?(zer0)
Comment on attachment 8786954 [details] [diff] [review]
Avoid accessing dead windows in test-firefox-windows.js

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

::: addon-sdk/source/test/windows/test-firefox-windows.js
@@ +293,2 @@
>  
> +  assert.equal(windows.length, 1, "Correct number of browser windows");

Maybe we should also check that the window left open, is the one expected to be – just to be a more similar version of the check we had before; where we checked specifically that window 2 and window 3 where closed.

But I'm fine either way.
Attachment #8786954 - Flags: review?(zer0) → review+
(In reply to Matteo Ferretti [:zer0] [:matteo] from comment #3)
> Comment on attachment 8786954 [details] [diff] [review]
> Avoid accessing dead windows in test-firefox-windows.js
> 
> Review of attachment 8786954 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: addon-sdk/source/test/windows/test-firefox-windows.js
> @@ +293,2 @@
> >  
> > +  assert.equal(windows.length, 1, "Correct number of browser windows");
> 
> Maybe we should also check that the window left open, is the one expected to
> be – just to be a more similar version of the check we had before; where we
> checked specifically that window 2 and window 3 where closed.
> 
> But I'm fine either way.

Thanks for the quick review, I'll update with your suggestion before landing.

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0cfb84806c08
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.