677002 - Various docshell tests no longer work correctly with WindowSnapshot converted to SpecialPowers

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Boris Zbarsky [:bzbarsky]]

There are tests using WindowSnapshot in chrome windows. Unfortunately, SpecialPowers only gets set up for content tabs.  Even more unfortunately, bug 652494 means that the tests are happily passing without testing anything as a result.  :(

This is a regression from bug 666643, so luckily a recent-ish regression.  But in the meantime, we're missing test coverage...  Requesting tracking for his regression.

Comment 1

[Boris Zbarsky [:bzbarsky]]

I can fix this for tests using docshell_helpers.js by adding SpecialPowers to the imports array.  But there may well be other tests that need a working WindowSnapshot in a chrome window as well...

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

interesting.  I wonder why I haven't seen this failing.  I need to add SpecialPowers to docshell_helpers.js for a waitForFocus patch.  I can look through the chrome tests which use snapshotWindow and verify the snapshots are correct and there are no silent failures or ignored tests.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> I wonder why I haven't seen this failing. 

Because the tests pass; they just don't actually test anything.  Again, see bug 652494.

Maybe we should grep the output that we do log for exceptions in chrome tests for this particular failure?  Not sure whether the logging actually logs the exception message...

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

I verified these tests all work fine:
content/xul/content/test/test_bug398289.html
editor/libeditor/text/tests/test_bug636465.xul
layout/generic/test/test_bug469774.xul
modules/libpr0n/test/mochitest/test_animSVGImage.html
toolkit/components/places/tests/chrome/test_favicon_annotations.xul
toolkit/content/tests/chrome/bug451286_window.xul

This test I needed to edit, but already had edited in one of my working patches:
chrome/layout/base/test/chrome/test_chrome_content_integration.xul (specifically fixed in my patch)

Comment 5

[Asa Dotzler [:asa]]

release drivers aren't going to be tracking this.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> release drivers aren't going to be tracking this.

Why not?  We no longer have test coverage on a good chunk of docshell stuff, so have no idea whether things have regressed....

Joel, this looks like it dropped off your plate.  :(

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Back out the non-SpecialPowers parts of

Comment 8

[Boris Zbarsky [:bzbarsky]]

Actually, just taking.  We need to fix this, on trunk and aurora, so we have something like test coverage.

I'm just backing out bug 666643 for now until it can be done right.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: fix windowsnapshot calls in chrome (1.0)

this patch fixes the docshell tests and 1 layout test which call snapshotwindow from chrome instead of mochitest plain.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 562022 [details] [diff] [review]
fix windowsnapshot calls in chrome (1.0)

I don't think this is the right approach.  I'll comment on what I think is the right thing in bug 666643 when I reopen it...

Comment 11

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/76d641ec4a44

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 561954 [details] [diff] [review]
Back out the non-SpecialPowers parts of

Asking for aurora approval to restore test coverage there.

Comment 13

[Ed Morley [:emorley]]

(In reply to Boris Zbarsky (:bz) from comment #11)
> http://hg.mozilla.org/integration/mozilla-inbound/rev/76d641ec4a44

https://hg.mozilla.org/mozilla-central/rev/76d641ec4a44

Comment 14

[Asa Dotzler [:asa]]

Comment on attachment 561954 [details] [diff] [review]
Back out the non-SpecialPowers parts of

go for it. approval-mozilla-beta+

Comment 15

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1cc47e765a5b

Comment 16

[u279076]

qa- as nothing to do for QA fix verification -- please reset to qa+ if there is something for QA to verify.