handle overflow for orthogonal blocks

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

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.
Posted file 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.
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.
Attachment #8597705 - Flags: review?(smontagu)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
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.)
Perhaps it's more appropriate to do this where we initially create the nsHTMLReflowState, rather than as a post-creation fixup?
Attachment #8597731 - Flags: review?(dbaron)
Attachment #8597705 - Attachment is obsolete: true
Attachment #8597705 - Flags: review?(smontagu)
(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 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.
Attachment #8597731 - Flags: review?(dbaron) → review-
(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?
Attachment #8600863 - Flags: review?(dbaron) → review+
Here's a reftest to go with this, which fails without the patch because the paragraph after the orthogonal block disappears.
Attachment #8600899 - Flags: review?(dbaron)
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
Attachment #8600899 - Flags: review?(dbaron) → review+
Indeed it does, with several thousand differing pixels. :)
Attachment #8597731 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/40c49a5de4e5
https://hg.mozilla.org/mozilla-central/rev/42484455a383
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Depends on: 1215787
You need to log in before you can comment on or make changes to this bug.