Closed Bug 1010344 Opened 6 years ago Closed 6 years ago
Comparing windows deeply makes no sense
There are SDK tests that do things like: assert.notDeepEqual(windowUtils.activeBrowserWindow, browserWindow); This is somewhat nonsensical for two reasons: 1) Treating two window objects as equal id they happen to quack the same in terms of properties even though they're different windows makes no sense. 2) This code is relying on a Gecko bug: the .window property on those objects is not an own property when viewed via Xrays. Once that's fixed, this code goes into an infinite loop any time the two windows have the same sets of own property names. Fix coming up.
Comment on attachment 8422548 [details] [diff] [review] Don't do deep compares on window objects in addon SDK tests, since that's not a very useful thing to do and can go into infinite loops Review of attachment 8422548 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this is dumb
Attachment #8422548 - Flags: review?(dtownsend+bugmail) → review+
Commit pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/d92602ec91ca0c3c87517d72ee84b84a17dbcbb6 Bug 1010344. Don't do deep compares on window objects in addon SDK tests, since that's not a very useful thing to do and can go into infinite loops. r=mossop
Backed out because something in the push was causing mochitest crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/2b99b42f1337 https://tbpl.mozilla.org/php/getParsedLog.php?id=39747566&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=39747507&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=39747838&tree=Mozilla-Inbound
I was going to reland this (since the crashes were from bug 1004458) but looks like it landed with the SDK uplift (and _boy_ did that confuse blame).
Sorry about that, I saw it went in and we were doing an uplift so I made sure it got included in our repo
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
It's not an issue, apart from blame being very confused.
You need to log in before you can comment on or make changes to this bug.