Closed Bug 971655 Opened 10 years ago Closed 10 years ago

Containing box of <fieldset> with <legend> for absolute positioning has the "bottom:0" point below the bottom of the <fieldset>


(Core :: Layout: Positioned, defect)

Not set



Tracking Status
firefox29 --- fixed
firefox30 --- fixed


(Reporter: masayuki, Assigned: MatsPalmgren_bugz)



(Keywords: regression)


(2 files, 1 obsolete file)

Attached image screenshot
As I filed bug 971653, the top edge of containing box of <fieldset> with <legend> for absolute positioning is different from other browsers.

According to this testcase, the containing box is just moved down without size adjustment.

The internal <div> element has "top: 0; bottom: 0; left: 0; right: 0;". So, the <div> (red dashed border's box) represents the containing box size.

Even if bug 971653 is INVALID or WONTFIX, I believe that this is a bug.
Here's a somewhat-more-reduced testcase:

data:text/html,<fieldset style="position:relative; height: 100px; width:100px;"><legend>l</legend><div style="position:absolute; bottom: 0">abspos

The abspos text has "bottom:0", so it should be (just) inside the fieldset border, but instead it's pushed out (by the <legend>).

mozregression says this behavior changed with this window:
Last good nightly: 2013-10-25
First bad nightly: 2013-10-26


So: (unsurprisingly) caused by bug 261037.
Blocks: 261037
Keywords: regression
Summary: Containing box of <fieldset> with <legend> for absolute positioning is overflowed from <fieldset> → Containing box of <fieldset> with <legend> for absolute positioning has the "bottom:0" point below the bottom of the <fieldset>
Attached patch wip (obsolete) — Splinter Review
We do compensate the height of the inner frame for the legend offset here:
but then CalculateContainingBlockSizeForAbsolutes adds in the padding
for the FieldsetFrame here (since they have the same content):
although it's the inner (-moz-fieldset-content) frame that is the
abs.pos. containing block.
Blocks: 971933
No longer blocks: 971933
Attached patch fix+reftestSplinter Review
Assignee: nobody → matspal
Attachment #8375123 - Attachment is obsolete: true
Attachment #8375761 - Flags: review?(roc)
Comment on attachment 8375761 [details] [diff] [review]

Review of attachment 8375761 [details] [diff] [review]:

::: layout/generic/nsBlockFrame.cpp
@@ +895,5 @@
>      const nsHTMLReflowState* aLastRS = &aReflowState;
>      const nsHTMLReflowState* lastButOneRS = &aReflowState;
>      while (aLastRS->parentReflowState &&
> +           aLastRS->parentReflowState->frame->GetContent() == frame->GetContent() &&
> +           aLastRS->parentReflowState->frame->GetType() != nsGkAtoms::fieldSetFrame) {

Please add a comment explaining why we're doing this.
Attachment #8375761 - Flags: review?(roc) → review+
Added a code comment:
Flags: in-testsuite+
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Might be worth uplifting this and bug 971933 to Aurora.  What do you think?
Flags: needinfo?(roc)
Comment on attachment 8375761 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 261037
User impact if declined: wrong layout for abs.pos. children in <fieldset>
Testing completed (on m-c, etc.): on m-c since 2014-02-15
Risk to taking this patch (and alternatives if risky): low risk
String or IDL/UUID changes made by this patch: none
Attachment #8375761 - Flags: approval-mozilla-aurora?
Attachment #8375761 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.