Closed Bug 1163565 Opened 10 years ago Closed 9 years ago

[css-grid] Percentage padding/margin for abs.pos. boxes with grid container containing block should use the CB height

Categories

(Core :: Layout: Positioned, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1163435
Tracking Status
firefox41 --- affected

People

(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Follow-up from bug 1163435. I've posted to www-style@ to get confirmation on this: https://lists.w3.org/Archives/Public/www-style/2015May/0154.html
I think the problem is the use of IsFlexOrGridItem() here: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsHTMLReflowState.cpp#1929 bool nsIFrame::IsFlexOrGridItem() const { if (GetParent()) { nsIAtom* t = GetParent()->GetType(); return (t == nsGkAtoms::flexContainerFrame || t == nsGkAtoms::gridContainerFrame) && !(GetStateBits() & NS_FRAME_OUT_OF_FLOW); } return false; } Which excludes abs.pos. frames. We should probably just check the parent type explicitly in BlockDirOffsetPercentBasis and not use IsFlexOrGridItem there. Daniel, do you want me to change it for abs.pos. flex items as well? I'm guessing we should use the same rule there - assuming www-style confirms this of course.
Flags: needinfo?(dholbert)
Attached patch wip (obsolete) (deleted) — Splinter Review
Yep, this seems to fix it for grid.
(In reply to Mats Palmgren (:mats) from comment #1) > Daniel, do you want me to change it for abs.pos. flex items as well? > I'm guessing we should use the same rule there - assuming www-style confirms > this > of course. Yup, we should be consistent, if www-style agrees that this is what's actually intended. (which I'm not sure about)
Flags: needinfo?(dholbert)
OK. FYI, it seems we have no flexbox tests for this since they all passed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=78de820de451
IE Edge matches our current behavior on this, FWIW -- they resolve vertical percent margin/padding against container *width*, for abspos things in flex containers. Tested by comparing these two URLs (which just differ on whether the child is abspos): http://jsfiddle.net/vh6m41e2/ http://jsfiddle.net/vh6m41e2/2/
Jet says this was resolved yesterday by the CSSWG - it should be calculated against height for both flex/grid items including abs.pos.
Attached patch fix (deleted) — Splinter Review
Attachment #8608848 - Flags: review?(dholbert)
Attachment #8604058 - Attachment is obsolete: true
(In reply to Mats Palmgren (:mats) from comment #6) > Jet says this was resolved yesterday by the CSSWG - it should be calculated > against height for both flex/grid items including abs.pos. Are you sure? I read through the CSSWG meeting notes[1] about percent resolution in flexbox/grid, and I didn't see any obvious discussion of this applying to abspos children. My impression was that the WG resolved to leave things as they are in the spec -- and the current spec text on this is pretty clear that... * This special case (vertical % applying to height) applies to flex items and grid items (see 2nd paragraph of flexbox spec section 4.2 [2] & grid spec section 2.3 [3]). * Abspos children of a [flex|grid] container are *not* [flex|grid] items. (this distinction is made in various places throughout both specs). [1] http://krijnhoetmer.nl/irc-logs/css/20150519#l-837 timestamp [21:06] through [22:32] [2] http://dev.w3.org/csswg/css-flexbox-1/#item-margins [3] http://dev.w3.org/csswg/css-grid-1/#grid-area-concept
I asked Jet specifically about abs.pos. and he said it was discussed and that it should be the same. Anyway, I guess someone will answer my www-style query (comment 0) soon, that will hopefully clarify it.
Comment on attachment 8608848 [details] [diff] [review] fix (Canceling review until we've got clarification about this on www-style. Failing that, I think this patch goes against the spec for now, per bullet points in comment 9.)
Attachment #8608848 - Flags: review?(dholbert)
Blocks: 1000592
I'm fixing this together with in-flow items in bug 1163435.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: