Closed
Bug 775779
Opened 12 years ago
Closed 12 years ago
social sidebar tests are marked as "leaking" the sidebar document
Categories
(Firefox Graveyard :: SocialAPI, defect)
Firefox Graveyard
SocialAPI
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 17
People
(Reporter: Gavin, Assigned: markh)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
See bug 775380. This applies to browser/base/content/test/browser_social_sidebar.js (and browser/base/content/test/browser_social_mozSocial_API.js which I'll be adding in bug 773351).
This seems similar to bug 759711 and/or bug 728426. The tests trigger the unhiding of the sidebar, which loads a URL in its browser. When the sidebar is hidden again, the sidebar browser is hidden and set to load about:blank.
For the moment I've just disabled the tests in debug builds. Hopefully bug 728294 will help with this? I'm not sure.
Comment 1•12 years ago
|
||
Now that bug 728426 has been fixed I think this leak could be gone. You still might be blamed for leaking the sidebar's outer window as long as bug 728294 hasn't landed but that should still be within the leak threshold.
Comment 2•12 years ago
|
||
(In reply to Tim Taubert [:ttaubert] from comment #1)
> Now that bug 728426 has been fixed I think this leak could be gone.
Nope. For some to-me-unknown reason, the social UI doesn't use the standard sidebar infrastructure but has its own sidebar on the right side of the browser.
Reporter | ||
Comment 3•12 years ago
|
||
Yes, we'll need to port bug 728426's fix to the social sidebar code.
Assignee | ||
Comment 4•12 years ago
|
||
This patch is on top of the patch in bug 773351 - it creates a new AboutBlankContentViewer before resetting to about:blank and re-enables the test on debug builds. I'll request review once 773351 lands.
Assignee | ||
Comment 5•12 years ago
|
||
This version also removes the "skip if debug" check from browser_social_sidebar.js
Attachment #645144 -
Attachment is obsolete: true
Attachment #647076 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #647076 -
Attachment is obsolete: true
Attachment #647076 -
Flags: review?(gavin.sharp)
Attachment #647138 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 7•12 years ago
|
||
Comment on attachment 647138 [details] [diff] [review]
Don't enable test in mozSocial_api due to bug 778639
*sigh* - it's not clear that this patch has any effect on the reported leaks in current builds. However, IMO this should be a priority so we can get all our tests running in debug builds to prevent things like the "compartment mismatch" issue screwing us.
Attachment #647138 -
Flags: review?(gavin.sharp) → review-
Assignee | ||
Comment 8•12 years ago
|
||
It appears bug 728294 prevented these "leaks" from being reported. I pushed the attachment to try at https://tbpl.mozilla.org/?tree=Try&rev=fb7acc56fb24 and the builds that have already finished look green (except for one orange that looks unrelated).
Attachment #647138 -
Attachment is obsolete: true
Attachment #649989 -
Flags: review?(gavin.sharp)
Reporter | ||
Comment 9•12 years ago
|
||
Comment on attachment 649989 [details] [diff] [review]
Avoid skipping social tests in debug builds.
I noticed this recently too! (https://tbpl.mozilla.org/?tree=Try&rev=d222940df46c)
Attachment #649989 -
Flags: review?(gavin.sharp) → review+
Reporter | ||
Comment 10•12 years ago
|
||
Target Milestone: --- → Firefox 17
Comment 11•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•