448132 - last set of y1/y4 tests in acid3 test 46 (media queries) fail on Windows

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P1)

Description

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

The last set of y1/y4 tests in test 46 of acid3 fail on Windows only.  I'm guessing it's something related to the delayed resizing stuff or hidden visibility for when we size windows to 0x0.

This is tested in both http://acid3.acidtests.org/ and layout/style/test/test_acid3_test46.html .

Comment 1

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

The test changes that should be reverted when this is fixed are:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/4dee7a278399
http://hg.mozilla.org/mozilla-central/index.cgi/rev/d0e438d64ea0

Comment 2

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

Commenting out this code fixes the problem:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/windows/nsWindow.cpp&rev=3.745&mark=4880-4886#4879

Comment 3

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

The code in question was added for bug 56156:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/widget/src/windows/nsWindow.cpp&mark=3.297#3.297

Comment 4

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

So I think this chunk of code needs a better test for being minimized than (newHeight==0 && newWidth==0).  There's in fact a big chunk of code right below the code causing the problem that deals with size mode changes.  (Interestingly, the code that's causing the problem seems like it probably *also* causes us to skip that code.)

It would be great if I could talk somebody else into figuring out what the right thing to do here is...

Comment 5

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

And note that while commenting out the code I cite in comment 2 (which fixes this bug) does not currently cause bug 56156, it does currently cause incorrect resizes to 0x0 (as shown by putting a printf at the start of PresShell::ResizeReflow), so we should have something here.

Comment 6

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

To be clear, what I was trying to say in comment 5 is that we do still need code to suppress the resizes that result from being minimized.  We just need to fix that code so that it doesn't suppress other resizes.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Rob would love to work on this.

Comment 8

[Rob Arnold [:robarnold]]

Attachment: v1.0

This does exactly what David suggested in comment 6. I'm not sure why there are minimized child windows at all

Comment 9

[Gabriele Best [:Gabri]]

Ready to be landed?

Comment 10

[Rob Arnold [:robarnold]]

I'm working on getting the test changes reverted and then I'll verify that it does fix the problem.

Comment 11

[Rob Arnold [:robarnold]]

Attachment: v1.0 w/tests

I verified that my fix does fix the test. I'll push when the tree reopens.

Comment 12

[Gabriele Best [:Gabri]]

The tree is already open http://tinderbox.mozilla.org/Firefox/

Comment 13

[Rob Arnold [:robarnold]]

Ok, pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/af50e2f9c650

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

This seems to have caused bug 454473 -- the regression is fairly bad, potentially bad enough to cause this to be backed out.

Comment 15

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

Sounds like the patch didn't do comment 6 correctly, then.

Comment 16

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I wonder if we're getting a 0x0 resize before the iconic state is set on the window?  That would cause it, I think.

Comment 17

[Rob Arnold [:robarnold]]

I'm not sure why IsIconic wouldn't have the right return value, but we could try checking wp->flags for SWP_HIDEWINDOW or call ::GetWindowPlacement as is done just a bit further down. The former is easier, but I suspect the latter may be more correct.

Comment 18

[Martijn Wargers (dead)]

This patch should be backed out, since it caused bug 454473 (blocking1.9+) and bug 452794.

Comment 19

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

For the record, this patch has been modified in bug 454473 and again in bug 502713.