Vertical center alignment using margin:auto for absolutely positioned elements does not work when the container is shorter than the element

RESOLVED DUPLICATE of bug 812899

Status

()

Core
Layout: R & A Pos
RESOLVED DUPLICATE of bug 812899
3 years ago
a year ago

People

(Reporter: d.zakharov, Assigned: bz)

Tracking

({reproducible, testcase})

33 Branch
reproducible, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
Created attachment 8523484 [details]
comparison.png

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.111 Safari/537.36

Steps to reproduce:

Copy the following into a text file and open with Firefox:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8" />
    <title>My Bug </title>
    <style>
        #large-container {
            height:200px;
            background-color:yellow;
            position:relative;
        }
        .my-element {
            height:50px;
            background-color:grey;
            position:absolute;
            top:0px;
            bottom:0px;
            margin:auto;
        }
        #small-container {
            background-color:cyan;
            height:25px;
            position:relative;
        }
    </style>
</head>
<body id="exam-layout">
    <!-- works as expected -->
    <div id="large-container">
        <div class="my-element">Foo element</div>
    </div>
    <!-- does not align -->
    <div id="small-container">
        <div class="my-element">Foo element</div>
    </div>
</body>
</html>


Actual results:

The grey box is not centered with respect to the cyan box.


Expected results:

The grey box should be correctly centered with respect to the cyan box. This behavior works correctly within the yellow box, but stops working once the container becomes smaller than its absolutely positioned child. The screenshot shows a comparison of this code running in FF vs IE and Chrome. Both other browsers center the element.

Comment 1

3 years ago
Created attachment 8524792 [details]
Reporter's testcase

Updated

3 years ago
Attachment #8524792 - Attachment mime type: text/plain → text/html

Updated

3 years ago
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout: R & A Pos
Ever confirmed: true
Keywords: reproducible, testcase
OS: Windows 8.1 → All
Product: Firefox → Core
Hardware: x86_64 → All
Summary: Vertical center alignment for absolutely positioned elements does not work when the container is shorter than the element → Vertical center alignment using margin:auto for absolutely positioned elements does not work when the container is shorter than the element
Whiteboard: DUPEME?
The relevant spec here is http://www.w3.org/TR/CSS21/visudet.html#abs-non-replaced-height which says:

  If none of the three ['top', 'bottom', 'height'] are 'auto': If both 'margin-top' and
  'margin-bottom' are 'auto', solve the equation under the extra constraint that the two
  margins get equal values. 

Looking at nsHTMLReflowState::InitAbsoluteConstraints it has this bit:


1640     if (marginTopIsAuto) {
1641       if (marginBottomIsAuto) {
1642         if (availMarginSpace < 0) {
1643           // FIXME: Note that the spec doesn't actually say we should do this!
1644           ComputedPhysicalMargin().bottom = availMarginSpace;
1645         } else {
1646           // Both 'margin-top' and 'margin-bottom' are 'auto', so they get
1647           // equal values
1648           ComputedPhysicalMargin().top = availMarginSpace / 2;
1649           ComputedPhysicalMargin().bottom = availMarginSpace - ComputedPhysicalMargin().top;
1650         }

and we're precisely in this "FIXME" case.

David, you introduced this case in http://hg.mozilla.org/mozilla-central/rev/52512f6d14c9 as far as I can tell (fix for bug 419100).  Though I guess the preexisting behavior was about the same as well.

It seems to me like the answer to bug 419100 comment 4 should be "code change" in this case, right?
Created attachment 8526189 [details] [diff] [review]
When an abs pos element has non-auto offsets and height but auto top/bottom margins, make the top/bottom margins equal even when the total margin space is negative
Attachment #8526189 - Flags: review?(dbaron)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Whiteboard: DUPEME?
Comment on attachment 8526189 [details] [diff] [review]
When an abs pos element has non-auto offsets and height but auto top/bottom margins, make the top/bottom margins equal even when the total margin space is negative

I guess I think the spec is wrong here, but given that IE and Blink match it per bug 419100 comment 7, r=dbaron
Attachment #8526189 - Flags: review?(dbaron) → review+
OK, there's one test failure.  It's happening because for our standalone image viewer we actually use auto margins on an abs pos element (the image) to center it, but now it starts overflowing above the viewport.  Caught by dom/html/test/test_bug369370.html

And looking at the spec, it seems like there's a fundamental asymmetry here between width and height: for the former the start margin is not allowed to go negative.  Now I see why you think the spec is wrong.

Anyway, is there a way to get the behavior we want for the image viewer while still making this change, short of computing stuff in JS?  It doesn't seem to me like there is...
Flags: needinfo?(dbaron)
I don't see anything that we currently implement.

Probably implementing something with the 'safe' keyword in:
http://dev.w3.org/csswg/css-align/#overflow-values

(I currently see implementing most of that spec as a relatively high-priority item, although we probably need to push the spec a bit to stabilize.)
Flags: needinfo?(dbaron)
Hmm.  Maybe we should hold off on this patch until we do that, then.

The other option would be making the behavior here depend on whether the containing block has visible overflow, but that seems like unnecessary magic.
This got fixed in bug 812899, where the test failure was independently rediscovered and dealt with....
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 812899
You need to log in before you can comment on or make changes to this bug.