469331 - [SeaMonkey] test_bug458898.html fails

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Serge Gautherie (:sgautherie)]

This test was added by bug 458898 comment 34.

It fails on SeaMonkey:
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1229072592.1229076992.23839.gz
Linux comm-central dep unit test on 2008/12/12 01:03:12

*** 38854 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug458898.html | innerWidth
*** 38855 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug458898.html | innerHeight

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1229072592.1229078831.28064.gz
MacOSX 10.4 comm-central dep unit test on 2008/12/12 01:03:12

*** 38881 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug458898.html | innerWidth
*** 38882 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug458898.html | innerHeight

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1229086183.1229090165.1689.gz
Win2k3 comm-central dep unit test on 2008/12/12 04:49:43

*** 38865 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/base/tests/test_bug458898.html | innerHeight
}

Comment 1

[Robert Kaiser]

The failure started right after this test was landed on 1.9.1 (which is what SeaMonkey is using), and the test fails consistently, on all platforms. The fix is supposed to be on the platform level, so I guess something actually still goes wrong there.

Comment 2

[Robert Kaiser]

When I execute the important command from the test:
window.openDialog("data:text/html,<div style='height:200px; width:100px;'>");
in Error Console in my current SeaMonkey build from today, I shortly see the window popping up (for a split second) but immediately disappearing again, leaving an item in the system's task bar, but no window that actually appears when trying to view it.

Comment 3

[Robert Kaiser]

Attachment: wallpaper fix: only install this test with Firefox for now

Looks like a real fix might not be coming within days right now, but I'd like us to get the SeaMonkey tree green over the holidays.
As Firefox os the only app we know atm that passes this test, this patch wallpapers over the problem and only installs the test there for now. I will not mark the bug here itself fixed if I can check this in, as we're not really passing the test yet but I hope that e.g. the fix for bug 469203 helps us there actually, we'll retry when that is on 1.9.1 and if that fixes us, we can re-enable this test everywhere.

roc reportedly is on vacation, so need someone else to review, I guess. dbaron, can you do that?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 353896 [details] [diff] [review]
wallpaper fix: only install this test with Firefox for now

Skipping the test seems like the wrong thing; instead you should call 'todo' instead of 'ok' inside the test, so then we'll know to enable it again when things are fixed.

(You can just write JS that sniffs the user agent and sets a variable to either ok or todo.)

Comment 5

[Robert Kaiser]

Sniffing the UA for Firefox is hairy due to different brand names, but sniffing for SeaMonkey and only going todo() for that should work reasonably - everyone else than those two is probably not running those tests right now in any case.

(I still wonder how Firefox passes this test as Minefield doesn't look correctly in any way on my machine when trying what the test does manually.)

Comment 6

[Robert Kaiser]

Attachment: mark the test todo on SeaMonkey [Checkin: Comment 8]

OK, here's the variant that marks the test todo on SeaMonkey only.

BTW, when I thought about UA sniffing being a bad way to deal with this and remembered that platform and app detection is even harder in xpcshell tests, I came to the conclusion to file bug 470557 on a way to make this easier for both.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 353976 [details] [diff] [review]
mark the test todo on SeaMonkey
[Checkin: Comment 8]

r=dbaron

Comment 8

[Robert Kaiser]

Thanks, pushed as http://hg.mozilla.org/mozilla-central/rev/7fdd82286b73 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/26335ebfd383

Comment 9

[Serge Gautherie (:sgautherie)]

Arf, this (2nd) patch failed to take into account that Windows (for whatever reason) was not failing the "ok(innerWidth)":
now, Windows is failing the "todo(innerWidth)" ;-<

{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1229882572.1229886905.4356.gz
Win2k3 comm-central dep unit test on 2008/12/21 10:02:52

*** 38912 ERROR TEST-UNEXPECTED-PASS | /tests/layout/base/tests/test_bug458898.html | innerWidth
}

Comment 10

[Robert Kaiser]

Attachment: make the width test function always ok on Windows [Checkin: Comment 12]

This is not really nice, but then, this whole product-specific test function screwing is ugly from the beginning.
This patch makes the width test function always OK on Windows, that should cover the unexpected pass we're seeing there atm.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 354295 [details] [diff] [review]
make the width test function always ok on Windows
[Checkin: Comment 12]

r=dbaron

Comment 12

[Robert Kaiser]

Pushed the Windows test fix as http://hg.mozilla.org/mozilla-central/rev/823540b5353a and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bba8ff78b8a2

Comment 13

[Serge Gautherie (:sgautherie)]

verified1.9.1, as the Windows box does not report this any of these errors anymore.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Did a bug ever get filed to investigate what's really happening here?

Comment 15

[Robert Kaiser]

(In reply to comment #14)
> Did a bug ever get filed to investigate what's really happening here?

I heavily expect bug 465993 and/or bug 469203 behind this, if those don't help, we really should investigate it further.

Comment 16

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081230 SeaMonkey/2.0a3pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/6201c0669e15
 +http://hg.mozilla.org/comm-central/rev/2854a16a867f)

V.Fixed

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> When I execute the important command from the test:
> window.openDialog("data:text/html,<div style='height:200px; width:100px;'>");
> in Error Console in my current SeaMonkey build from today, I shortly see the
> window popping up (for a split second) but immediately disappearing again,

Is it still the case ?

*****

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081230 SeaMonkey/2.0a3pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/6201c0669e15
 +http://hg.mozilla.org/comm-central/rev/2854a16a867f)

This (m-c) build include bug 469203 fix.

I don't know what the behavior was "before", but here is what it is currently:
*Firefox  : window looks fine, innerWidth=117 innerHeight=216 :-)
*SeaMonkey: window looks fine, innerWidth=104 innerHeight=0   :-(

So the reported innerHeight is obviously broken...

Maybe one of these bugs:
https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc_type=anywordssubstr&short_desc=innerWidth+innerHeight&resolution=---

Comment 18

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) Report the values [Checkin: Comment 21 & 23]

It helps to have the actual value.

Comment 19

[Robert Kaiser]

Please don't hijack fixed bugs for more work, filing new bugs is cleaner for that and easier to track.

And, BTW, the build you mention in comment #17 is experimental, not normal "SeaMonkey trunk" (the latter would be using 1.9.1, but you use mozilla-central), you should mention things like that when replying to comments like mine that talk about "current" builds, which are clearly what default "SeaMonkey trunk" builds are.

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 355122 [details] [diff] [review]
(Cv1) Report the values
[Checkin: Comment 21 & 23]

r=dbaron

Comment 21

[Serge Gautherie (:sgautherie)]

Comment on attachment 355122 [details] [diff] [review]
(Cv1) Report the values
[Checkin: Comment 21 & 23]

http://hg.mozilla.org/mozilla-central/rev/6c260601ec86

Comment 22

[Serge Gautherie (:sgautherie)]

(In reply to comment #19)
> Please don't hijack fixed bugs for more work, filing new bugs is cleaner for
> that and easier to track.

I see no hijack here:
you did a (disabling) workaround;
I'm enhancing the test to confirm explicitly the values on all 3 platforms;
_then_ I'll file a new bug !

> And, BTW, the build you mention in comment #17 is experimental, not normal
> "SeaMonkey trunk" (the latter would be using 1.9.1, but you use
> mozilla-central), you should mention things like that when replying to comments

I thought "rv:1.9.2a1pre + mozilla-central" was enough,
but, agreed, I'll add "experimental" too until we get SM/m-c tinderboxes.

> like mine that talk about "current" builds, which are clearly what default
> "SeaMonkey trunk" builds are.

That's why I asked you for an update: I don't build 1.9.1...

Comment 23

[Serge Gautherie (:sgautherie)]

Comment on attachment 355122 [details] [diff] [review]
(Cv1) Report the values
[Checkin: Comment 21 & 23]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fef0ad965b31

Comment 24

[Robert Kaiser]

(In reply to comment #15)
> (In reply to comment #14)
> > Did a bug ever get filed to investigate what's really happening here?
> 
> I heavily expect bug 465993 and/or bug 469203 behind this, if those don't help,
> we really should investigate it further.

That suspicion unfortunately didn't hold true so I filed bug 472886 on that.

Comment 25

[Serge Gautherie (:sgautherie)]

(In reply to comment #24)
> I filed bug 472886 on that.

Eventually duped to bug 471959, which I had filed after comment 22 :-|