917032 - Refactor flexbox layout logic some more, to make pagination more straightforward

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

I split out some of my flexbox-pagination code into some simpler minor-refactoring patches, which my pagination patches stack on top of.

(This is much like bug 885424, just with a few new patches.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: part 1: do flex layout in container's content-box space

So right now, nsFlexboxFrame runs the flexbox layout algorithm using coordinates that are relative to its border-box.

This turned out to make things a bit tricky for pagination patches, because it means there are several places where we need GetSkipSides() checks to find out whether the container's border/padding should be discounted.

SO: This patch makes things simpler by doing flexbox layout in content-box space -- i.e. *ignoring* borders and padding on the container -- and then adding them in at the last minute, when we give each child their final reflow.

Comment 2

[Daniel Holbert [:dholbert]]

(Note: The two "EnterMargin(ReflowState.mComputedBorderPadding)" calls removed in Part 1 are what currently move our PositionTracker "current position" representation past the flex container's border/padding, so that flex items end up inside of the border/padding instead of on top of it.  Since the patch makes us ignore the container's border/padding until the very end, it can also drop those calls.)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 2: Make PhysicalPositionFromLogicalPosition() take content-box size and flip polarity if necessary

This patch abstracts some logic into PhysicalPositionFromLogicalPosition().

If we have a right-to-left flexbox that's 200px wide, and we end up with a flex item at main-axis-position 50px, we need to convert that "50px from right edge" into "x-position of 200px-50px = 150px" to get x-position where we should reflow the child.

This patch just pushes that polarity-flipping code into PhysicalPositionFromLogicalPosition(), to make Reflow a little cleaner/clearer.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 3: move some code up a little

This patch just moves some code up a bit -- in particular, it makes us compute a flex item's "physical" (x,y) position in its container *before* we create its nsReflowState.

Right now, it doesn't matter where we do this computation, but it'll be needed in the earlier spot for pagination support, because when we switch to giving the flex item's reflow state a meaningful availableHeight, we'll need to have its physical y-position in order to compute the right available height to give it.

Comment 5

[Daniel Holbert [:dholbert]]

Green try run w/ these 3 patches: https://tbpl.mozilla.org/?tree=Try&rev=9410251ffa4a

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 805768 [details] [diff] [review]
part 1: do flex layout in container's content-box space

These changes looks good to me.  r=mats

Comment 7

[Daniel Holbert [:dholbert]]

Thanks! Landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3a3f570a81e
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e76e17797e8
https://hg.mozilla.org/integration/mozilla-inbound/rev/67cf1e541846

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c3a3f570a81e
https://hg.mozilla.org/mozilla-central/rev/9e76e17797e8
https://hg.mozilla.org/mozilla-central/rev/67cf1e541846