626395 - In print mode, content hidden via overflow:hidden still impacts the layout

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P1)

Description

[Florens Verschelde :fvsch]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9) Gecko/20100101 Firefox/4.0b9
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9) Gecko/20100101 Firefox/4.0b9

I have found two different buggy behaviours with the print rendering of pages with content hidden via overflow:hidden.
This set of pages demonstrates it (control page, bug 1, bug 2):
http://covertprestige.info/bugs/ff4-print-overflow/
Each test page is self-containing, no external CSS or JS.

- When the overflowing content is normal text, it's hidden but still takes some space and generates empty pages.
- When the overflowing content is a single, non-breakable string of text, the full width of the string seems to be used for adapting the zoom level of the printed page. I have not reproduced this test with different non-breakable content (e.g. an image, or a large display:block element), but such tests could be added to a test-case.

Note that this problem happens with generated content as well (see http://fvsch.com/).

Finally, in my tests this issue could not be reproduced on different browsers, including Firefox 3.6.

Reproducible: Always

Comment 1

[Florens Verschelde :fvsch]

Attachment: Copy of the 3 test pages

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is probably a regression from bug 129941.  The setup there works ok for scroll/auto, but for hidden the paint-time clipping is really not right....

roc, dbaron, can we get around this by just setting the overflow area to the box bounds for this case (hidden overflow while printing)?  Or would that have other bad effects?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That should work.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Seems like nsIFrame::FinishAndStoreOverflow should check the same things that ApplyOverflowClipping does, though it might not need to check ApplyOverflowHiddenClipping given what nsFrame::ConsiderChildOverflow does.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(and maybe also nsBlockFrame::ComputeOverflowAreas)

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, excellent.  I'll experiment a bit tomorrow.  I do think this needs to block.

Comment 7

[Boris Zbarsky [:bzbarsky]]

OK.  So doing that doesn't seem to help.  And the reason it doesn't seem to help is that the inner div decides it's taller than the available height and splits.  This forces its parent div to also split, etc.  So while the overflow areas are equal to the frame bounds, we still end up creating multiple pages.

The overflow area changes _do_ fix the scaling thing, though.  roc, is there a decent way to test that?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: What I have so far

Comment 9

[Boris Zbarsky [:bzbarsky]]

Oh, and actually using overflow:-moz-hidden-unscrollable directly has the same problem with the extra pages.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Is there a way to make the block just claim to be complete once its non-auto height is used up?  That seems like it would be ideal... the question is what to do with stuff that's hanging out on overflow lists at that point, right?

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think if an overflow:not-visible block's remaining computed height fits in the available height, it should tell its children that they have unlimited available height. That will let them all be placed on that page.

Comment 12

[fantasai]

Hm, I think what you want to do when the non-auto height is used up on an overflow:hidden block is to make the available height unconstrained. The only caveat I have there is to make sure forced page breaks don't crash, as they might be confused by this. (I wouldn't worry too much about getting the forced page breaks to work correctly, as they need to be ripped out and rewritten. Just make sure they don't land the frame tree in some kind of unstable state.)

Comment 13

[Boris Zbarsky [:bzbarsky]]

> The only caveat I have there is to make sure forced page breaks don't crash

nsPageBreakFrame::Reflow says:

  aDesiredSize.height = (aReflowState.availableHeight == NS_UNCONSTRAINEDSIZE ?
                        0 : aReflowState.availableHeight);

So I think we're ok there.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

This seems to do the trick.  I don't know how to write an automated test for the scaling issue, sadly.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 505086 [details] [diff] [review]
Proposed fix

+  nsHTMLReflowState reflowState(aReflowState);
+  // If we have non-auto height, we're clipping our kids and we fit,
+  // make sure our kids fit too.

I think we should avoid stack-allocating and copying the reflow state here in the common case. In the rare case that the new reflow state is needed, heap-allocate one.

Comment 16

[Boris Zbarsky [:bzbarsky]]

> In the rare case that the new reflow state is needed, heap-allocate one.

Done.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/6f0219923a28

Florent, thanks for the bug report and the excellent testcases!

Comment 18

[Aleksei Perepelkin [:chaos]]

Verified with Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre

Comment 19

[u279076]

Works for me on Windows and Mac too.