Marcia Knous [:marcia] Reporter
	Description • 17 years ago

Seen using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2008010904 Minefield/3.0b3pre.

STR:
1. Go to http://mozillazine.org/ and print the first page, as part of the Litmus smoketest. Print to the page to a Epson Stylus C88+ local printer.

Expected:

The page would print correctly, with the columns on the left and right hand side of the pages showing on the printed page.

Actual:

The page does not print correctly. The columns on the left and right are stripped from the first page, and in print preview I can only see remnants of part of the column on the top of the second page.

Safari is able to print the first page fine (including columns), and shows a total of 9 pages in preview.

	juan becerra [:juanb]
	Comment 1 • 17 years ago

Compare this with Fx2 which prints the page properly.

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 2 • 17 years ago

It seems that not only mozillazine is affected. A lot of pages are missing parts when printing them. Sounds like a bug with major or even higher severity. Asking for blocking1.9.

	Daniel Holbert [:dholbert]
	Comment 3 • 17 years ago

I see this on Linux as well.
OS/Platform --> All/All

	Daniel Holbert [:dholbert]
	Comment 5 • 17 years ago

As stated in a comment within testcase 1:

<!-- Note: There are two ways you can tweak this testcase to make the text
  -- visible in print-preview:
  --  A. Reduce height of filler to 500px, so it doesn't overflow first page.
  --  B. Put filler *after* sidebar. (Note that this doesn't change the normal
  --      browser rendering, though it fixes print-prev)  -->

	Daniel Holbert [:dholbert]
	Comment 6 • 17 years ago

Here's testcase 1 again, with whitespace removed from the body for nicer framedumps.

	Daniel Holbert [:dholbert]
	Comment 7 • 17 years ago

This reference case just has the order of the divs switched. (tweak B from Comment 5)

	Daniel Holbert [:dholbert]
	Comment 8 • 17 years ago

This semi-reference case uses a shorter filler div, less than the height of a page. (tweak A from Comment 5)

	Daniel Holbert [:dholbert]
	Comment 9 • 17 years ago

I think the root of the problem is this:  In testcase 1, the absolutely-positioned div's placeholder is pushed onto the second page.  However,  the absolutely-positioned div itself falls on the first page.  So we have a placeholder that points to a div on another page, which (I think) isn't supposed to happen.

(This problem doesn't happen in reference 1 because the placeholder comes first, and it doesn't happen in testcase 2 because filler isn't tall enough to push the placeholder all the way onto the second page.)

	Daniel Holbert [:dholbert]
	Comment 10 • 17 years ago

Could someone track down a regression range for when testcase 1 broke?
[ adding qawanted keyword ]

	Daniel Holbert [:dholbert]
	Comment 11 • 17 years ago

This bug is *old*.

I just tracked down a (large) regression range of between 2005-10-01 and 2006-02-01 for testcase1 -- have to go now, though, so I can't narrow it further at the moment.

Note that you may need to increase the height of the "filler" div (say, to 2000px) in order to reproduce the bug in older builds (near early 2007, I think).  We apparently used to be able to cram 1000 pixels onto the first page. (due to different unit-conversion rules or something)

	Daniel Holbert [:dholbert]
	Comment 12 • 17 years ago

Ok, regression range is between:
  2006-01-25
  2006-01-26
Bonsai link: http://tinyurl.com/32qj76
I'm guessing this is a regression from Bug 317375 ("Reorganize frame painting and mouse event targeting around frame display lists").

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 13 • 17 years ago

stealing

	Daniel Holbert [:dholbert]
	Comment 15 • 17 years ago

See also these testcases from bug 426399, which have the same regression range as this bug:
 testcase 4a:  attachment 313459 [details]
 testcase 4b:  attachment 313460 [details]
 reference 4:  attachment 313461 [details]

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 16 • 17 years ago

I've got a fix for this, just need to make some reftests. It fixes all the testcases in bug 426399 too.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 17 • 17 years ago

The comments in nsLayoutUtils should explain what's going on. Basically when painting a single page in nsLayoutUtils::PaintFrame we need to traverse other pages to find placeholders through which we should paint abs-pos frames on the current page. This code does that.

PruneDisplayListForExtraPage, which is the trickiest thing in this patch, isn't *really* needed. It does two things:
1) eliminate display items that belong only to the extra page(s)
2) fix up clipping coordinates

Issue 1) doesn't really arise since we pass an empty dirty rect to create the display list for the extra pages, so the only frames for which we will create display items are ones marked NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO when we constructed the display list for the current page.

Issue 2) doesn't arise AFAIK since the only way I know of to get an element with clipping on page 2 which could still draw a clipped child on page 1 would be to use an abs-pos element with 'clip:rect(-1000px ...)' or similar, but clip only applies to abs-pos elements and bug 426909, which will not be fixed for 1.9, prevents display of abs-pos elements that don't start on the first page.

So I'm going to attach a simpler patch that does none of those things.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 18 • 17 years ago

Comment on attachment 313485 [details] [diff] [review]
fix

On second thoughts I think we should go with this. Relying on the dirty rect would be fragile and probably would fail with container frames with borders and backgrounds that contain placeholders for frames on earlier pages.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 19 • 17 years ago

To get this approach to work I had to prevent page-content frames from clipping everything outside the page --- that directly makes them abs-pos frames on earlier pages they contain placeholders for. So I moved the page-content clipping out and consolidated it with existing clipping in nsPageFrame.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 20 • 17 years ago

The way I'd *like* this all to work as part of a big printing rework plan is:
-- Making printing place each printed page so that page-content edges are adjacent.
-- When printing each page, construct a display list for the whole document at once with the current page selected as the dirty rect.
-- Rework print preview as we've discussed elsewhere (not sure where :-( ) ... Make all the print-preview bling outside the page contents part of a print-preview XUL document, and each page-contents is a special XUL element that renders a page directly from the print presentation.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 21 • 17 years ago

dholbert, you probably want to look at this patch too.

	David Baron :dbaron:
	Comment 22 • 17 years ago

Comment on attachment 313485 [details] [diff] [review]
fix

>+      // We may need to paint out-of-flow frames whose placeholders are
>+      // on other pages. Add those pages to our display list. Note that
>+      // out-of-flow frames can't be placed after their placeholders so
>+      // we don't have to process earlier pages. The display lists for
>+      // these extra pages are pruned so that only display items for the
>+      // page we currently care about (which we would have reached by
>+      // following placeholders to their out-of-flows) end up on the list.

I'm having trouble following this.  I can understand that an out-of-flow can be after its placeholder (e.g., when there are a lot of floats and some end up on later pages), but I don't see how it can end up before.  Did you just say this the wrong way around (i.e., s/after/before/)?  Or am I missing something?

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 23 • 17 years ago

Actually I said it the right way around.

Abs-pos frames are always children of the first-in-flow of their containing block, even though the placeholders may be pushed to be descendants of continuations of the containing block.

For floats, the placeholder and the out-of-flow's first-in-flow must always be descendants of the same continuation frame of the float containing block. For example, the code around PushTruncatedPlaceholderLine ensures that if a line has a placeholder for a below-current-line float, but the BCL float needs to be pushed to the next page, we actually split the line before that placeholder to ensure that it goes with the BCL float to the next page.

	David Baron :dbaron:
	Comment 24 • 17 years ago

Comment on attachment 313485 [details] [diff] [review]
fix

>+ * @param aPage the page we constructed aList for

Could you call this aExtraPage to match terms used elsewhere?  (The variable too, not just the comment.)

>+        // This might clip an element which should appear on the first
>+        // page, and that element might be visible if this uses a 'clip'

s/first page/main page/ ?  (For consistent terminology.)

>+        // property with a negative top.

Is this interpretation of 'clip' across pages sanctioned by any specs?  I suppose it vaguely makes sense, but it's the kind of thing that the spec ought to describe if this is how it works.  Any chance you could raise this on www-style if it matches what other browsers do?

>+static nsIFrame*
>+GetNextPage(nsIFrame* aPageContentFrame)
>+{
>+  // XXX ugh
>+  nsIFrame* pageFrame = aPageContentFrame->GetParent();
>+  nsIFrame* nextPageFrame = pageFrame->GetNextSibling();
>+  if (!nextPageFrame)
>+    return nsnull;
>+  return nextPageFrame->GetFirstChild(nsnull);

Some assertions about frame types would be nice here.

r+sr=dbaron

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 25 • 17 years ago

(In reply to comment #24)
> Is this interpretation of 'clip' across pages sanctioned by any specs?  I
> suppose it vaguely makes sense, but it's the kind of thing that the spec ought
> to describe if this is how it works.  Any chance you could raise this on
> www-style if it matches what other browsers do?

Breaking of absolutely-positioned elements across pages is not described in the spec. I guess I can raise the 'clip' issue but this room is full of elephants.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 26 • 17 years ago

Checked in

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 27 • 17 years ago

Pages getting printed correctly now. So the first page contains the sidebars and text and each further page only the text => Verified.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041506 Minefield/3.0pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008041504 Minefield/3.0pre ID:2008041504