Bug 70793 - Need to handle absolutely positioned elements inside flexboxes
Summary: Need to handle absolutely positioned elements inside flexboxes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks: 62048 70792
  Show dependency treegraph
 
Reported: 2011-10-24 19:23 PDT by Ojan Vafai
Modified: 2012-01-10 15:53 PST (History)
6 users (show)

See Also:


Attachments
Patch (22.90 KB, patch)
2011-12-29 12:10 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (22.97 KB, patch)
2011-12-29 15:07 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (28.82 KB, patch)
2012-01-09 16:15 PST, Tony Chang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2011-10-24 19:23:35 PDT
We also need to review this part of the spec to make sure it makes sense: http://dev.w3.org/csswg/css3-flexbox/#abspos-flexbox-items.
Comment 1 Tony Chang 2011-12-29 12:10:47 PST
Created attachment 120763 [details]
Patch
Comment 2 Tony Chang 2011-12-29 12:50:35 PST
I didn't add test cases for row-reverse/column-reverse to this patch because I wasn't sure what it was supposed to do.  It'll still be out of flow, but it's not clear to me where they should be positioned.  For example:

<div style="display: -webkit-flexbox; -webkit-flex-flow: row-reverse; position: relative">
<div style="position: absolute;">absolute</div>
<div style="width: 1000px">main text</div>
</div>

+---------------------------------------+
|(1)main text                        (2)+(3)
+---------------------------------------+

It's not clear to me if absolute should be at 1, 2 (right aligned) or 3.  I guess (2) makes the most sense, but it will require some extra logic to handle properly (i.e., should be a separate patch).
Comment 3 Tab Atkins 2011-12-29 13:28:26 PST
The static position is the upper-right corner of the "main text" div (flush with the inner cross-start edge of the flexbox, and flush with the main-start edge of the following neighbor).  Abspos elements put their top-left corner at their static position, so (3) is correct.
Comment 4 Tony Chang 2011-12-29 15:07:22 PST
Created attachment 120769 [details]
Patch
Comment 5 WebKit Review Bot 2011-12-29 15:56:02 PST
Comment on attachment 120769 [details]
Patch

Attachment 120769 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10924136

New failing tests:
http/tests/inspector/resource-tree/resource-tree-document-url.html
Comment 6 Tony Chang 2012-01-05 16:04:00 PST
Comment on attachment 120769 [details]
Patch

The spec page hasn't been updated to the consensus from TPAC.  The conclusion was to have position:absolute flex items insert a placeholder.  All this means is that for flex-pack: justify, the spacing is a bit weird.

I'll update the patch to do this instead.
Comment 7 Tab Atkins 2012-01-06 08:36:03 PST
I'm about to go look through the minutes, but I don't recall that actually being the decision.  You may want to hold off on making the change immediately while I verify.
Comment 8 Ojan Vafai 2012-01-06 11:36:22 PST
(In reply to comment #7)
> I'm about to go look through the minutes, but I don't recall that actually being the decision.  You may want to hold off on making the change immediately while I verify.

That definitely was the conclusion. I think it's silly though. dbaron argued that leaving behind a placeholder makes it consistent with absolute positioning in the rest of the platform (e.g. an absolutely positioned div inside an inline).
Comment 9 Tab Atkins Jr. 2012-01-06 13:35:44 PST
(In reply to comment #8)
> (In reply to comment #7)
> > I'm about to go look through the minutes, but I don't recall that actually being the decision.  You may want to hold off on making the change immediately while I verify.
> 
> That definitely was the conclusion. I think it's silly though. dbaron argued that leaving behind a placeholder makes it consistent with absolute positioning in the rest of the platform (e.g. an absolutely positioned div inside an inline).

Ah, indeed.  Looks like I remembered wrong.  Welp, time to go change the spec.
Comment 10 Ojan Vafai 2012-01-07 12:59:17 PST
The text has been updated http://dev.w3.org/csswg/css3-flexbox/#abspos-flexbox-items.

We should make sure to add a test for "they'll...join neighboring inline elements in their anonymous flexbox item wrapper boxes."
Comment 11 Tab Atkins Jr. 2012-01-07 13:48:44 PST
(In reply to comment #10)
> The text has been updated http://dev.w3.org/csswg/css3-flexbox/#abspos-flexbox-items.
> 
> We should make sure to add a test for "they'll...join neighboring inline elements in their anonymous flexbox item wrapper boxes."

You probably want three tests, for the following situations:

<flexbox>
  <div />
  <abspos />
  text
</flexbox>

<flexbox>
  text
  <abspos />
  <div />
</flexbox>

<flexbox>
  <div />
  text
  <abspos />
  text
  <div />
</flexbox>

Using 'flex-pack:justify' for all three of them to make it easier to tell what's happening.
Comment 12 Tony Chang 2012-01-09 16:15:35 PST
Created attachment 121750 [details]
Patch
Comment 13 Dave Hyatt 2012-01-10 13:28:15 PST
Comment on attachment 121750 [details]
Patch

r=me
Comment 14 WebKit Review Bot 2012-01-10 15:53:13 PST
Comment on attachment 121750 [details]
Patch

Clearing flags on attachment: 121750

Committed r104645: <http://trac.webkit.org/changeset/104645>
Comment 15 WebKit Review Bot 2012-01-10 15:53:19 PST
All reviewed patches have been landed.  Closing bug.