No horizontal scrollbar with element inside fieldset and padding-top

VERIFIED FIXED in mozilla1.9beta2



13 years ago
12 years ago


(Reporter: martijn.martijn, Assigned: roc)


({regression, testcase})

Windows XP
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [dbaron-1.9:RwCo])


(4 attachments, 1 obsolete attachment)



13 years ago
See upcoming testcase.

You should see a horizontal scrollbar also with the testcase.
And the bottom scrollbarbutton should work normally.
This is not the case with current trunk builds.
This regressed between 2006-01-25 and 2006-01-26, making this a likely regression from bug 317375.

Comment 1

13 years ago
Posted file testcase


12 years ago
Flags: blocking1.9?
Assignee: nobody → roc
Flags: blocking1.9? → blocking1.9+
Posted file another testcase
This testcase does not rely on padding in the fieldset
Posted patch fix (obsolete) — Splinter Review
This wasn't really a display lists regression ... they just exposed some really bogus reflow code in nsFieldSetFrame (surprise!), namely:

     if (aReflowState.ComputedHeight() != NS_INTRINSICSIZE && + mLegendSpace+kidDesiredSize.height > aReflowState.ComputedHeight()) {
       kidDesiredSize.height = aReflowState.ComputedHeight()-( + mLegendSpace);

Problem #1 is subtracting the fieldset's border+padding from its content height and assuming that means something sensible. It doesn't.

Problem #2 is bashing the fieldset's anonymous block frame size without adjusting anything else, like its overflow area or the layout of its children. That's what causes weird problems in the testcases in this bug.

My solution is to change the computed height of the child *before* we reflow it to subtract out the "content space" taken up by the legend, if any --- which I think is what the bogus code is trying to do. This works very well.

This patch also removes lame attempts to handle margins on the fieldset's anonymous block, which are pointless because it can't have margins. Really handling margins would complicate the fix I'm implementing here.
Attachment #285991 - Flags: superreview?(bzbarsky)
Attachment #285991 - Flags: review?(bzbarsky)
Whiteboard: [needs review]
Comment on attachment 285991 [details] [diff] [review]

>Index: layout/forms/nsFieldSetFrame.cpp
>+    // Our child is "height:100%" but we actually want its height to be reduced
>+    // by the amount of content-height the legend is eating up.
>+    kidReflowState.SetComputedHeight(PR_MAX(0, aReflowState.ComputedHeight() - mLegendSpace));

If we're auto-height, the kid will be too, no?  That is, aReflowState.ComputedHeight() might be unconstrained.  You probably don't want to run this line in that case.

The rest of this looks good, but we need to address the unconstrained issue.
Attachment #285991 - Flags: superreview?(bzbarsky)
Attachment #285991 - Flags: superreview-
Attachment #285991 - Flags: review?(bzbarsky)
Attachment #285991 - Flags: review-
Posted patch good callSplinter Review
Sure wish we had saturating nscoords...
Attachment #285991 - Attachment is obsolete: true
Attachment #286058 - Flags: superreview?(bzbarsky)
Attachment #286058 - Flags: review?(bzbarsky)
Comment on attachment 286058 [details] [diff] [review]
good call

Attachment #286058 - Flags: superreview?(bzbarsky)
Attachment #286058 - Flags: superreview+
Attachment #286058 - Flags: review?(bzbarsky)
Attachment #286058 - Flags: review+
Whiteboard: [needs review] → [needs landing]
Whiteboard: [needs landing] → [needs landing][dbaron-1.9:RwCo]

Comment 7

12 years ago
Note to self, need to look whether this would fix the click problem I have with the "Zoek" button sometimes at
Priority: -- → P3
Checking in layout/forms/nsFieldSetFrame.cpp;
/cvsroot/mozilla/layout/forms/nsFieldSetFrame.cpp,v  <--  nsFieldSetFrame.cpp
new revision: 3.164; previous revision: 3.163
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing][dbaron-1.9:RwCo] → [dbaron-1.9:RwCo]
Target Milestone: --- → mozilla1.9 M10

Comment 9

12 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007110805 Minefield/3.0b2pre

Also the problem I had at seems fixed.
checked in test.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.