Closed Bug 626395 Opened 9 years ago Closed 9 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla2.0b10
Tracking Status
blocking2.0 --- final+

People

(Reporter: fvsch, Assigned: bzbarsky)

References

(Depends on 1 open bug, )

Details

(Keywords: regression, Whiteboard: [softblocker][fx4-fixed-bugday])

Attachments

(2 files, 1 obsolete file)

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
Component: General → Printing: Output
Product: Firefox → Core
QA Contact: general → printing
Version: unspecified → Trunk
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?
Blocks: 129941
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Keywords: regression
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.
(and maybe also nsBlockFrame::ComputeOverflowAreas)
OK, excellent.  I'll experiment a bit tomorrow.  I do think this needs to block.
Assignee: nobody → bzbarsky
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
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?
Attached patch What I have so far (obsolete) — Splinter Review
Oh, and actually using overflow:-moz-hidden-unscrollable directly has the same problem with the extra pages.
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?
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.
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.)
> 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.
Attachment #504997 - Attachment is obsolete: true
Attached patch Proposed fixSplinter Review
This seems to do the trick.  I don't know how to write an automated test for the scaling issue, sadly.
Attachment #505086 - Flags: review?(roc)
Whiteboard: [softblocker] → [need review][softblocker]
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.
Attachment #505086 - Flags: review?(roc) → review+
> In the rare case that the new reflow state is needed, heap-allocate one.

Done.
Whiteboard: [need review][softblocker] → [need landing][softblocker]
Pushed http://hg.mozilla.org/mozilla-central/rev/6f0219923a28

Florent, thanks for the bug report and the excellent testcases!
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [need landing][softblocker] → [softblocker]
Target Milestone: --- → mozilla2.0b10
Blocks: 609227
Verified with Mozilla/5.0 (X11; Linux i686; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre
Works for me on Windows and Mac too.
Status: RESOLVED → VERIFIED
Whiteboard: [softblocker] → [softblocker][fx4-fixed-bugday]
Depends on: 1414603
You need to log in before you can comment on or make changes to this bug.