layout: Allow inline elements to be containing blocks for absolutely-positioned elements.

[pcwalton]

r? @mbrubeck

[highfive]

Warning

• These commits modify unsafe code. Please review it carefully!

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/4869

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[bors-servo]

☔ The latest upstream changes (presumably #5916) made this pull request unmergeable. Please resolve the merge conflicts.

[mbrubeck]

^{components/layout/flow.rs, line 1328 [r1] (raw file):}
There's code in multicol.rs that still uses the old name for this fn, causing a compiler error.

---

---

Comments from the review on Reviewable.io

[bors-servo]

☔ The latest upstream changes (presumably #5935) made this pull request unmergeable. Please resolve the merge conflicts.

[mbrubeck]

Reviewed files:

• components/layout/layout_task.rs @ r4
• components/layout/list_item.rs @ r4
• components/layout/multicol.rs @ r4
• components/layout/table.rs @ r1
• components/layout/table_caption.rs @ r1
• components/layout/table_cell.rs @ r1
• components/layout/table_colgroup.rs @ r1
• components/layout/table_row.rs @ r1
• components/layout/table_rowgroup.rs @ r4
• components/layout/table_wrapper.rs @ r4
• tests/ref/absolute_inline_containing_block_a.html @ r1
• tests/ref/absolute_inline_containing_block_ref.html @ r1
• tests/ref/basic.list @ r4

---

^{tests/ref/basic.list, line 45 [r4] (raw file):}
All but two of these files are missing.

---

Comments from the review on Reviewable.io

[mbrubeck]

Reviewed files:

• components/gfx/display_list/mod.rs @ r4
• components/layout/block.rs @ r4

---

Comments from the review on Reviewable.io

[pcwalton]

It was merge fallout. Rebased and fixed. r? @mbrubeck

[mbrubeck]

Reviewed files:

• components/layout/data.rs @ r1
• components/layout/display_list_builder.rs @ r4

---

^{components/layout/display_list_builder.rs, line 1494 [r4] (raw file):}
Should this be combined with the previous case using |?

---

Comments from the review on Reviewable.io

[mbrubeck]

Reviewed files:

• components/gfx/display_list/mod.rs @ r5
• components/layout/block.rs @ r5
• components/layout/construct.rs @ r5
• components/layout/display_list_builder.rs @ r5
• components/layout/layout_task.rs @ r5
• components/layout/table.rs @ r5
• components/layout/table_wrapper.rs @ r5
• tests/ref/basic.list @ r5

---

Comments from the review on Reviewable.io

[mbrubeck]

Reviewed files:

• components/layout/flow.rs @ r5
• components/layout/fragment.rs @ r5

---

^{components/layout/flow.rs, line 1381 [r5] (raw file):}
Please add a doc comment.

---

^{components/layout/fragment.rs, line 1659 [r5] (raw file):}
Can this be combined with the preceding pattern using |?

---

^{components/layout/fragment.rs, line 1824 [r5] (raw file):}
Should this be combined with the preceding match arm?

---

Comments from the review on Reviewable.io

[mbrubeck]

Reviewed files:

• components/layout/inline.rs @ r5

---

^{components/layout/inline.rs, line 1458 [r5] (raw file):}
This is O(n^2) in the worst case. If that's a concern, this loop could record indices as it goes, though it might get tricky if you can have multiple fragments from the same flow that are not consecutive...

---

^{components/layout/inline.rs, line 1467 [r5] (raw file):}
This looks unreachable to me, because we just found an InlineAbsolute fragment for this flow in self.fragments. Isn't containing_block_range_for_flow guaranteed to find the same one or an earlier one?

---

^{components/layout/inline.rs, line 1581 [r5] (raw file):}
Nit: The result local isn't needed here.

---

Comments from the review on Reviewable.io

[mbrubeck]

Review complete; some minor comments on Reviewable above.

[pcwalton]

^{components/gfx/display_list/mod.rs, line 544 [r3] (raw file):}
fasdfsadf

---

^{components/layout/fragment.rs, line 1659 [r5] (raw file):}
Unfortunately not, because InlineBlockFragmentInfo and InlineAbsoluteFragmentInfo are different types.

---

Comments from the review on Reviewable.io

[mbrubeck]

I can reproduce the test failure locally on Linux; the box is offset downward 1px in absolute_inline_containing_block_a.html.

[bors-servo]

☔ The latest upstream changes (presumably #5969) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably #6011) made this pull request unmergeable. Please resolve the merge conflicts.

[mbrubeck]

Reviewed files:

• components/layout/block.rs @ r8
• components/layout/construct.rs @ r7
• components/layout/fragment.rs @ r7, r8
• components/layout/inline.rs @ r7, r8
• components/layout/layout_task.rs @ r7
• components/layout/table.rs @ r8
• components/layout/table_cell.rs @ r8
• components/layout/table_row.rs @ r8
• components/layout/table_rowgroup.rs @ r8
• components/layout/table_wrapper.rs @ r8
• tests/ref/basic.list @ r7, r8

---

Comments from the review on Reviewable.io

[pcwalton]

@bors-servo: r=mbrubeck

[bors-servo]

📌 Commit a299a2c has been approved by mbrubeck

[bors-servo]

⌛ Testing commit a299a2c with merge c0ee952...

[bors-servo]

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2

[nox]

@pcwalton @mbrubeck Could one of you explain why do we need to check if any fragment is positioned in inline elements, instead of just checking whether the inline element itself is positioned?

[mbrubeck]

If I remember correctly, this is because there is not a 1:1 mapping between iinline elements and inline flows. A single inline flow may contain fragments from multiple elements. When traversing the flow tree, one can't simply look at "the inline element" of an inline flow.