Closed Bug 948654 Opened 7 years ago Closed 7 years ago
flex items not wrapping in multi-line flexbox with "overflow:hidden"
This was originally reported in bug 939901 comment 48. STR: Load https://bugzilla.mozilla.org/attachment.cgi?id=8345511 in Firefox Nightly. EXPECTED RESULTS: Bar with "EDIT" & "DELETE" at the bottom of each "card". ACTUAL RESULTS: That bar is not visible; instead, it's rendered as a partially-visible gray bar at the right edge of each card, clipped from "overflow:hidden". As noted in the testcase's source, you'll get EXPECTED RESULTS if you remove "overflow:hidden". Chrome (Blink) 32.0.1685.0 dev and Opera (Presto) 12.16 both show EXPECTED RESULTS.
Summary: flex items → flex items not wrapping in nested multi-line flexbox with "overflow:hidden"
A simpler test case, i.e. stripped down to the core problem which is a *single* wrapping flex box layout, not wrapping with hidden overflow.
Summary: flex items not wrapping in nested multi-line flexbox with "overflow:hidden" → flex items not wrapping in multi-line flexbox with "overflow:hidden"
Thanks, Patrick! That's helpful. Even further-reduced testcase: data:text/html,<div style="display: flex; flex-wrap: wrap; overflow: hidden; width: 30px; border: 1px solid black;"><div>a</div><div>b</div><div>c</div><div>d</div> Reference case: data:text/html,<div style="display: flex; flex-wrap: wrap; width: 30px; border: 1px solid black;"><div>a</div><div>b</div><div>c</div><div>d</div> I think the problem here is that we need to inherit "flex-wrap" on -moz-scrolled-content, here: http://mxr.mozilla.org/mozilla-central/source/layout/style/ua.css#128 Otherwise, the "flex-wrap" property is only seen on the "outer" scrollframe, and not on the inner scrolled flexbox, which means "flex-wrap" doesn't actually end up having any effect.
Ah yes, I even have a commented-out line there :) > 148 /* flex-wrap: inherit; FIXME: not yet supported (bug 702508) */ http://mxr.mozilla.org/mozilla-central/source/layout/style/ua.css#148 That just needs uncommenting.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
(Fortunately, that line (and the 'align-content' line a bit upwards from it) are the only instances of '702508' in the codebase -- so I don't have any other straggling TODOs associated with that bug.)
This still needs tests, but this should do it for the fix.
I intend to request backport approval to land this (trivial) patch on aurora, too. --> flagging as tracking-firefox28 to be sure it doesn't slip off the radar.
Here's the same fix, now with reftests for [flex-wrap, align-content] in both vertical and horizontal flex containers. (These tests are just hg cp'd from existing reftests that check for other flex-container properties being inherited through a scroll frame. The testcase has content with & without 'overflow' set; the reference case does not use 'overflow' at all, but instead tweaks the sizing to match the testcase's clipped overflow-region.)
Comment on attachment 8345632 [details] [diff] [review] fix v1 (now with reftests) r=mats (and I agree we should take this on Aurora)
Attachment #8345632 - Flags: review?(matspal) → review+
Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/9a8ca1c0e114 needinfo=me to request aurora approval once this has stuck on inbound and/or central.
Priority: -- → P3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment on attachment 8345632 [details] [diff] [review] fix v1 (now with reftests) Requesting aurora approval. This is a very minimal change (just uncommenting two lines of CSS that I intended to uncomment as part of fixing bug 702508) [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 702508 (feature) User impact if declined: Misrendered web content (Two CSS properties ignored, if they're on an "overflow:hidden" element) Testing completed (on m-c, etc.): local testing, 2 days on inbound, 1 day on m-c Risk to taking this patch (and alternatives if risky): Very low. String or IDL/UUID changes made by this patch: None.
Attachment #8345632 - Flags: approval-mozilla-aurora?
Comment on attachment 8345632 [details] [diff] [review] fix v1 (now with reftests) low risk, approving for landing, since it's trivial i'm going to take off the tracking flag and just set status flags.
Attachment #8345632 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Landed on aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/0fa5f4f29f7f
You need to log in before you can comment on or make changes to this bug.