Closed Bug 971655 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Layout: Positioned, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30
Tracking Status
firefox29 --- fixed
firefox30 --- fixed

People

(Reporter: masayuki, Assigned: mats)

References

Details

(Keywords: regression)

Attachments

(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.
http://jsfiddle.net/d_toybox/X2U8J/3/

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

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9f8233fcce1d&tochange=ef3f5669b53e
}

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:
http://hg.mozilla.org/mozilla-central/annotate/879038dcacb7/layout/forms/nsFieldSetFrame.cpp#l457
but then CalculateContainingBlockSizeForAbsolutes adds in the padding
for the FieldsetFrame here (since they have the same content):
http://hg.mozilla.org/mozilla-central/annotate/879038dcacb7/layout/generic/nsBlockFrame.cpp#l893
although it's the inner (-moz-fieldset-content) frame that is the
abs.pos. containing block.
No longer blocks: 971933
Attached patch fix+reftestSplinter Review
https://tbpl.mozilla.org/?tree=Try&rev=1de5f2f4bdf2
Assignee: nobody → matspal
Attachment #8375123 - Attachment is obsolete: true
Attachment #8375761 - Flags: review?(roc)
Comment on attachment 8375761 [details] [diff] [review]
fix+reftest

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:
https://hg.mozilla.org/integration/mozilla-inbound/rev/af811e70b7ad
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/af811e70b7ad
Status: NEW → RESOLVED
Closed: 6 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]
fix+reftest

[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.