Closed Bug 402872 Opened 17 years ago Closed 16 years ago

Hang with floating thead and large margin

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: bernd_mozilla)

References

Details

(Keywords: hang, regression, testcase)

Attachments

(4 files, 1 obsolete file)

Attached file testcase2
This is a testcase with fieldset and legend with min-height. It seems related because it has the same regression range.
This is a hang at nsBlockReflowState::CanPlaceFloat

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockReflowState.cpp&rev=3.540&mark=678-703#670 it never exits the for loop
which starts with a non saturated nsMargin.h 

http://mxr.mozilla.org/mozilla/source/gfx/public/nsMargin.h
and could be fixed by saying if there is infinite space then anything goes.

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsBlockReflowState.cpp&rev=3.540&mark=638#617
Attached patch patch (obsolete) — Splinter Review
Attachment #305375 - Flags: review?(dholbert)
Blocks: 367673
OS: Windows XP → All
Hardware: PC → All
Comment on attachment 305375 [details] [diff] [review]
patch

>+  if (NS_UNCONSTRAINEDSIZE == mAvailSpaceRect.height)
>+    return PR_TRUE;

Makes sense to me, with one exception... 

At one point, I was under the impression that nsRects should never have unconstrained height / width, because then YMost / XMost won't work correctly.  If that's the case, then this patch would just be a workaround rather than a fix.

Redirecting the review to roc, 'cause I think he was the one who I'd talked to about this before, and I think he knows.
Attachment #305375 - Flags: review?(dholbert) → review?(roc)
They do have unconstrained heights here, we'll just have to live with that :-(.

The problem is aFloatSize.height is greater than NS_UNCONSTRAINEDSIZE... So the patch is OK except I'd really like a new NSCoord helper, something like NSCoordLessThan(a, b) which asserts a != NS_UNCONSTRAINEDSIZE and tests a < b || b == NS_UNCONSTRAINEDSIZE, and use that here.
Attached patch revised patchSplinter Review
Attachment #305375 - Attachment is obsolete: true
Attachment #306779 - Flags: superreview?(roc)
Attachment #306779 - Flags: review?(roc)
Attachment #305375 - Flags: review?(roc)
Comment on attachment 306779 [details] [diff] [review]
revised patch

> NSCoordMoreThan

NSCoordGreaterThan

And please make the comments clear that "a" must not be unconstrained, but "b" can be.
Attachment #306779 - Flags: superreview?(roc)
Attachment #306779 - Flags: superreview+
Attachment #306779 - Flags: review?(roc)
Attachment #306779 - Flags: review+
Blocks: 420718
Attachment #307070 - Flags: approval1.9?
Comment on attachment 307070 [details] [diff] [review]
changed comment, function renamed

a1.9=beltzner
Attachment #307070 - Flags: approval1.9? → approval1.9+
Assignee: nobody → bernd_mozilla
fix checked in
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008030806
Minefield/3.0b5pre
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
crash tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/204befd400ff
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: