1307806 - [css-grid][css-flexbox][css-align] Implement nsLayoutUtils::GetFirstLineBaseline for flex/grid containers.

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Comment 1

[Mats Palmgren (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c0672b9107a7b5331a47f8b55b397c3621a6f7b

Comment 2

[Mats Palmgren (inactive)]

Attachment: fix

(don't worry about the test failures, they are unrelated)

Comment 3

[Mats Palmgren (inactive)]

BTW, we should probably add something to nsLayoutUtils::GetLastLineBaseline
as well eventually, but I don't think it matters for now.  Grid items that
are grid containers are handled specially in the Grid code for now, and
Flexbox layout doesn't implement last-baseline.

Comment 4

[Mats Palmgren (inactive)]

* yet. :-)

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 8798166 [details] [diff] [review]
fix

Review of attachment 8798166 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsLayoutUtils.cpp
@@ +6212,5 @@
> +        fType == nsGkAtoms::gridContainerFrame) {
> +      aResult->mBStart = 0;
> +      aResult->mBaseline = aFrame->GetLogicalBaseline(aWM);
> +      aResult->mBEnd = aFrame->BSize(aWM);
> +      return true;

This looks identical to the contents of the
  "if (fType == nsGkAtoms::tableWrapperFrame)"
case further up in this method.

Should we just share that existing code here, and just broaden its "if" check a bit?

Comment 6

[Daniel Holbert [:dholbert]]

(Also: can we include tests for this change?)

Comment 7

[Mats Palmgren (inactive)]

Attachment: fix

Hopefully bug 1306894 will be fixed soon enough so we'll get a test that way. ;-)

BTW, I asked Brad if he could take a look at that one - it seems like we should already have a baseline value somewhere in Flexbox layout that we can to store for that method to return, right?

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 8798207 [details] [diff] [review]
fix

Review of attachment 8798207 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

(Yeah, the flex container's baseline gets set (not-entirely-correctly) here:
>   aDesiredSize.SetBlockStartAscent(flexContainerAscent);
https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsFlexContainerFrame.cpp#4217

We could cache that on the frame in a member-variable, if that tends to match what we do elsewhere for storing baselines.)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32aa05912bbb
[css-grid][css-flexbox][css-align] Implement nsLayoutUtils::GetFirstLineBaseline for flex/grid containers.  r=dholbert

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/32aa05912bbb