733875 - absolutely-positioned content within a fixed-size relatively-positioned block will behave as if its container is zero-height, if we add a -moz-box grandparent

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

See attached testcase.  In both cases, we've got
 <grandparent>
   <parent: fixed-size, relatively-positioned div>
    <child: absolutely positioned child>

...both of which are blocks.  However -- if the grandparent is display:-moz-box (with no other style set), then the child ends up doing its absolute positioning differently.

In particular, it does its positioning such that "top" and "bottom" are offset from the same y-position, basically -- it behaves as if its parent has 0 height (even though the parent has a fixed-height of 200px).  The only reason you see any blue (for the child) in the attached testcase is because of the padding.  If you remove the padding, there's no blue.

ttaubert reported this as "testcase 5" on bug 640443, but I'm spinning it off to this bug because I think it's a separate issue from what bug 640443 describes.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 2: just specifying "bottom: 0"

(In reply to Daniel Holbert [:dholbert] from comment #0)
> Created attachment 603823 [details]
> testcase 1
> 
> See attached testcase.  In both cases, we've got
>  <grandparent>
>    <parent: fixed-size, relatively-positioned div>
>     <child: absolutely positioned child>
> 
> ...both of which are blocks.  However -- if the grandparent is
> display:-moz-box (with no other style set), then the child ends up doing its
> absolute positioning differently.
> 
> In particular, it does its positioning such that "top" and "bottom" are
> offset from the same y-position, basically -- it behaves as if its parent
> has 0 height.

Here's a similar testcase to demonstrate this in a different way.  In this version, there's no padding & no 'top' specification -- just 'bottom'.

In the reference (no -moz-box), this puts the blue block at the bottom of its container.

But in the -moz-box grandparent case, this puts the blue block just above its container. So -- it looks like "bottom" and "top" are offset w.r.t. the same y-position.

Comment 2

[Daniel Holbert [:dholbert]]

I believe this is needed for ttaubert's about:newtab work; setting blocking flag accordingly.

Comment 3

[Daniel Holbert [:dholbert]]

So, from the perspective of the parent (a fixed-size, relpos block): when it's inside of a -moz-box, it doesn't find out its size until it receives a call to child->SetBounds() here:
http://mxr.mozilla.org/mozilla-central/source/layout/xul/base/src/nsSprocketLayout.cpp#487
which is well after it has reflowed its positioned children.

So -- during reflow, the block doesn't know its size yet, and it ends up passing aContainingBlockWidth=0, aContainingBlockHeight=0 into its call to nsAbsoluteContainingBlock::Reflow, since that's what it's got as its desired size in its reflow metrics at that point.  So it ends up reflowing the child as if the containing block had zero height.

To fix this, I think we need to trigger another call to nsAbsoluteContainingBlock::Reflow _after_ the SetBounds call in nsSprocketLayout.cpp, or something like that.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yeah, welcome to the mess that is mixing multiple layout models!

Does this break percentage heights inside the block too, if you give the block a fixed height or whatnot?  Or does sprocket layout end up respecting that?

I really think that now that updating overflow areas doesn't need reflow we should move all abs pos reflow off to a post-reflow callback or equivalent so it happens once the geometry is fully known.  That would fix not only this case but also tables...

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (%-height instead of abspos)[not fixed by this bug]

(In reply to Boris Zbarsky (:bz) from comment #4)
> Does this break percentage heights inside the block too, if you give the
> block a fixed height or whatnot?

Yup, it totally does... gah.  See attached testcase.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Boris Zbarsky (:bz) from comment #4)
> I really think that now that updating overflow areas doesn't need reflow we
> should move all abs pos reflow off to a post-reflow callback or equivalent
> so it happens once the geometry is fully known.  That would fix not only
> this case but also tables...

That sounds pretty good.  I think we need a shorter-term fix for this, though, so the about:newtab rewrite can land. Patch coming up.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: fix v1

This is basically like the fix for bug 640443 -- adding a ReflowAbsoluteFrames call to DoLayout() -- but in this case it's the nsFrame version of DoLayout.  (which is the method inherited by our parent-block in this case).  This is late enough for us to have our size figured out.

Comment 8

[Daniel Holbert [:dholbert]]

(fix v1 doesn't help with percentages, of course, but it's nice & simple and it'll hopefully let bug 729878 move forward)

Tim, could you confirm whether this fixes the issue for you?  From my testing, it fixes testcases 1 & 2 here, and (combined with bug 640443's patch) it fixes your testcase 5 on bug 640443.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 603981 [details] [diff] [review]
fix v1

Awesome, thank you very much! Those patches make the new about:newtab layout finally work. I owe you a beer or something next time I'm in MV :)

Comment 10

[Daniel Holbert [:dholbert]]

Yay, that's great news!

I just need to sort out the assertion I mentioned in bug 640443 comment 33, and then we'll be good. Hopefully I can settle that tomorrow.

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: fix v2

This uses an unconstrained available height, like the latest patch in bug 640443, so we don't create continuation frames.  (In this case we probably _could_ create continuation frames, since the container is something block-like 
(Technically the container here could be any frame that establishes an abspos containing block & doesn't implement its own DoLayout method -- but I think we don't want continuation frames anyway, since our moz-box grandparent isn't splittable and our parent gets XUL DoLayout calls instead of Reflow calls.)

Comment 12

[Daniel Holbert [:dholbert]]

(er, sorry, didn't quite finish editing that comment before posting -- the gist is there, though.)

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 604179 [details] [diff] [review]
fix v2

r=me

Comment 14

[Daniel Holbert [:dholbert]]

Thanks! Landed:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/50ebad2e58ba

Comment 15

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/mozilla-central/rev/50ebad2e58ba