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

RESOLVED FIXED in Firefox 8

Status

Testing
Mochitest
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

({regression})

Trunk
mozilla9
regression
Points:
---

Firefox Tracking Flags

(firefox8- fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Blocks: 666643
(Assignee)

Comment 1

6 years ago
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...
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.
(Assignee)

Comment 3

6 years ago
> 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...
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

6 years ago
release drivers aren't going to be tracking this.
tracking-firefox8: ? → -
(Assignee)

Comment 6

6 years ago
> 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.  :(
Assignee: nobody → jmaher
(Assignee)

Comment 7

6 years ago
Created attachment 561954 [details] [diff] [review]
Back out the non-SpecialPowers parts of
Attachment #561954 - Flags: review?(khuey)
(Assignee)

Comment 8

6 years ago
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.
Assignee: jmaher → bzbarsky
Whiteboard: [need review]
Created attachment 562022 [details] [diff] [review]
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.
Attachment #562022 - Flags: review?(bzbarsky)
Attachment #561954 - Flags: review?(khuey) → review+
(Assignee)

Comment 10

6 years ago
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...
Attachment #562022 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 11

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/76d641ec4a44
Whiteboard: [need review]
Target Milestone: --- → mozilla9
(Assignee)

Comment 12

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

Asking for aurora approval to restore test coverage there.
Attachment #561954 - Flags: approval-mozilla-aurora?
(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
Status: NEW → RESOLVED
Last Resolved: 6 years ago
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Version: unspecified → Trunk

Comment 14

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

go for it. approval-mozilla-beta+
Attachment #561954 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
(Assignee)

Comment 15

6 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/1cc47e765a5b
status-firefox8: --- → fixed
qa- as nothing to do for QA fix verification -- please reset to qa+ if there is something for QA to verify.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.