Closed Bug 1580302 Opened 2 months ago Closed 2 months ago

XUL Box frames don't stretch as grid items

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Attached file testcase 1

STR:
(1) Apply bug 1580012's patch and set layout.css.xul-box-display-values.content.enabled to true [EDIT: fixed bug number]

(2) View attached testcase.

ACTUAL RESULTS:
The second item (the display:-moz-box one) is only as wide as its contents.

EXPECTED RESULTS:
That item should stretch to the full width of the grid (the default justify-self:stretch behavior).

Also, the last line of the testcase should report "[-moz-box]" -- if it says [block], then that's a hint that you didn't finish STR step (1).

It turns out this happens because we pass in ComputeSizeFlags::eShrinkWrap to ReflowInput's ComputeSize() call for this frame.

We do that because mFrameType (which has a version of the "display-outside" for the frame) is not block-flavored for this frame, and that means we end up initializing the computeSizeFlags to eShrinkWrap instead of eDefault here:
https://searchfox.org/mozilla-central/rev/588814f2edddf0e132d77d326ddae50911e8bad1/layout/generic/ReflowInput.cpp#2382-2385

      bool isBlock = NS_CSS_FRAME_TYPE_BLOCK == NS_FRAME_GET_TYPE(mFrameType);
      typedef nsIFrame::ComputeSizeFlags ComputeSizeFlags;
      ComputeSizeFlags computeSizeFlags =
          isBlock ? ComputeSizeFlags::eDefault : ComputeSizeFlags::eShrinkWrap;

If I add an exception to this for grid-item box frames, then I get EXPECTED RESULTS.

Note that if we changed XUL box frames to have DisplayOutside() report Block (emilio's approach in the try patches on bug 1580012), then this would Just Work per this chunk, I think:

    switch (disp->DisplayOutside()) {
      case StyleDisplayOutside::Block:
      case StyleDisplayOutside::TableCaption:
        frameType = NS_CSS_FRAME_TYPE_BLOCK;
        break;

https://searchfox.org/mozilla-central/rev/588814f2edddf0e132d77d326ddae50911e8bad1/layout/generic/ReflowInput.cpp#899-902

Here's a simple patch that should fix this (when combined with my initial patch in bug 1580012). Just posting to unblock ntim's experimentation for the moment.

As noted in prior comment: if we take a different approach on bug 1580012, then we may not need any additional patch.

Attachment #9091888 - Attachment description: wip.patch → WIP Patch: Don't force shrinkwrap codepath for box frames that are grid items.

I'm assuming this is going to be blocking the frontend work to remove XUL grids tracked in Bug 1520625.

Blocks: 1576946
Depends on: 1580012

Also: rename "isBlock" to "isBlockLevel" to reflect reality. Its naming is
based on the constant NS_CSS_FRAME_TYPE_BLOCK, which in fact is not specific to
display:block but in fact is for frames that are "block-level in normal flow"
(which I think in practice means block-level and not out-of-flow).

https://searchfox.org/mozilla-central/rev/7531325c8660cfa61bf71725f83501028178cbb9/layout/generic/ReflowInput.h#51

Attachment #9091888 - Attachment is obsolete: true
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/088c878f9f7c
Treat all flex items and grid items as being block-level in ReflowInput::InitConstraints. r=mats
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.