1152941 - handle overflow for orthogonal blocks

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[Jonathan Kew (:jfkthame)]

Attachment: testcase for orthogonal block with overflowing content

See testcase; reduce the width of the browser window so that the content of the vertical writing-mode <blockquote> does not all fit across the available width.

Currently, with overflow:visible we "lose" the rest of the text, although opening the element inspector makes it appear; OTOH, with overflow:hidden, the overflowing content is visible!

See http://dev.w3.org/csswg/css-writing-modes/#auto-multicol for more.

Comment 1

[Jonathan Kew (:jfkthame)]

Attachment: additional testcase

Another version of the testcase; again, observe the behavior when resizing the window small enough that the orthogonal <blockquote> element does not fit comfortably.

Comment 2

[Jonathan Kew (:jfkthame)]

Attachment: Always use unconstrained block-size when reflowing an orthogonal child block, to avoid incomplete reflow across orthogonal boundary

I think what we want to do here is to ensure that we don't try to fragment an orthogonal block and end up with overflow frames that we don't have any place to put. So in nsBlockReflowContext::ReflowBlock, force AvailableBSize to be unconstrained if the frame is orthogonal to its parent, so it won't end up incompletely reflowed. This makes the testcases here behave nicely.

Comment 3

[Jonathan Kew (:jfkthame)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=37cc2b55d9f8

Comment 4

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

Are there any other places there can be orthogonal boundaries that don't go through nsBlockReflowContext?

(It seems a little disturbing that this is a "fixup"; it's a pretty important invariant that we never set a constrained block-size unless we're paginating, and you'll likely get a bunch of security bugs if you violate it.)

Comment 5

[Jonathan Kew (:jfkthame)]

Attachment: Always use unconstrained block-size when reflowing an orthogonal child block, to avoid incomplete reflow across orthogonal boundary

Perhaps it's more appropriate to do this where we initially create the nsHTMLReflowState, rather than as a post-creation fixup?

Comment 6

[Jonathan Kew (:jfkthame)]

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #4)
> Are there any other places there can be orthogonal boundaries that don't go
> through nsBlockReflowContext?

Not that I'm aware of at this point, though it'd certainly be worth looking out for anything like that.

Comment 7

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

Comment on attachment 8597731 [details] [diff] [review]
Always use unconstrained block-size when reflowing an orthogonal child block, to avoid incomplete reflow across orthogonal boundary

I don't think an inline-block would go through either of these codepaths, yet I'd think an inline-block can be orthogonal to its parent.

I think this really needs the sort of fix that is guaranteed to cover all cases.

Comment 8

[Jonathan Kew (:jfkthame)]

(In reply to David Baron [:dbaron] ⏰UTC+2 from comment #7)
> Comment on attachment 8597731 [details] [diff] [review]
> Always use unconstrained block-size when reflowing an orthogonal child
> block, to avoid incomplete reflow across orthogonal boundary
> 
> I don't think an inline-block would go through either of these codepaths,
> yet I'd think an inline-block can be orthogonal to its parent.

Items with display:inline-block don't seem to be affected by the bug here: changing the <blockquote> in attachment 8597703 [details] to have display:inline-block makes it appear as expected.

(Because we set the AvailableBSize to unconstrained in nsLineLayout::ReflowFrame.)

> I think this really needs the sort of fix that is guaranteed to cover all cases.

We could do it in nsHTMLReflowState::Init, I suppose; does that cover things more thoroughly?

Comment 9

[Jonathan Kew (:jfkthame)]

Attachment: Always use unconstrained block-size when reflowing an orthogonal child block, to avoid incomplete reflow across orthogonal boundary

Like this, maybe?

Comment 10

[Jonathan Kew (:jfkthame)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c1e4d07e915d

Comment 11

[Jonathan Kew (:jfkthame)]

Attachment: Reftest for orthogonal block with potentially overflowing content

Here's a reftest to go with this, which fails without the patch because the paragraph after the orthogonal block disappears.

Comment 12

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

Comment on attachment 8600899 [details] [diff] [review]
Reftest for orthogonal block with potentially overflowing content

r=dbaron if you've checked that it fails without the patch

Comment 13

[Jonathan Kew (:jfkthame)]

Indeed it does, with several thousand differing pixels. :)

Comment 14

[Jonathan Kew (:jfkthame)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40c49a5de4e5
https://hg.mozilla.org/integration/mozilla-inbound/rev/42484455a383

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40c49a5de4e5
https://hg.mozilla.org/mozilla-central/rev/42484455a383