417676 - fixed positioned elements printed only on first and last pages

Status: RESOLVED FIXED

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

Description

[fantasai]

Attachment: testcase

Fixed positioning is broken in paginated mode: the box prints only on the first and last pages instead of on all pages. See testcase.

Comment 1

[fantasai]

This might be a regression from 389619: IsProperAncestorFrame doesn't return a match for a frame and the *continuation* of its ancestor.

Comment 2

[fantasai]

Ok, so the problem isn't that. This is a regression that goes back at least as far as Firefox 3 beta 1. It worked fine in FF2.

Comment 3

[fantasai]

Attachment: frametree after reflow

And the frametree looks correct after reflow...

Comment 4

[fantasai]

Fixed placeholder continuations are piling up at the end of the document, maybe that's the problem. The placeholder for the fixed frame on the middle page is inside the last page's document root continuation.

Comment 5

[fantasai]

Attachment: patch

Comment 6

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

Comment on attachment 304174 [details] [diff] [review]
patch

+   * Inserts aFrameList's frames into our main child list--without reparenting
+   * or requesting reflow.
+   */
+  virtual nsresult AddFrames(nsIFrame* aFrameList,
+                             nsIFrame* aPrevSibling);

The nsContainerFrame implementation requests reflow, so this comment is incorrect.

+nsPageContentFrame::IsFixedReplicatePlaceholder(nsIFrame* aFrame)

Shouldn't this just be IsFixedPlaceholder? You're not really testing "replicated".

+    nsFrameList cache = StealFixedPlaceholders(frame);

Not a good variable name.

Don't make a default parameter for StealFrame. Doesn't seem helpful.

This strategy could use better documentation in nsPageContentFrame::Reflow.

Is this really the simplest way to handle the problem? I guess the best solution would be to avoid requiring placeholders for duplicate fixed-pos elements?

Comment 7

[fantasai]

Attachment: patch v2

> The nsContainerFrame implementation requests reflow, so this comment is
> incorrect.

The nsContainerFrame implementation consists of mFrames.InsertFrames... Did you mean nsBlockFrame? What I meant was that it wasn't sending a request to the presshell like InsertFrames does. nsBlockFrame::AddFrames just marks the line box dirty; afaict it doesn't mark any frames dirty.

> Shouldn't this just be IsFixedPlaceholder? You're not really testing
> "replicated".

Good point. Fixed.

> Not a good variable name.

Changed to stolenPlaceholders.

> Don't make a default parameter for StealFrame. Doesn't seem helpful.

Removed. (Was getting compiler errors without it when I tried this in nsBlockFrame, but it compiles fine now.)

> This strategy could use better documentation in nsPageContentFrame::Reflow.

Added some comments

> Is this really the simplest way to handle the problem? 

I think so. I first attempted to handle it in nsBlockFrame::DrainOverflowLines, but that's a mess.

> I guess the best solution would be to avoid requiring placeholders for
> duplicate fixed-pos elements?

Probably. That would fix 'auto' positioning on pages after the first, too, but I don't think it's worth the trouble for 1.9.

Comment 8

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

Comment on attachment 304421 [details] [diff] [review]
patch v2

(In reply to comment #7)
> > The nsContainerFrame implementation requests reflow, so this comment is
> > incorrect.
> 
> The nsContainerFrame implementation consists of mFrames.InsertFrames... Did you
> mean nsBlockFrame? What I meant was that it wasn't sending a request to the
> presshell like InsertFrames does. nsBlockFrame::AddFrames just marks the line
> box dirty; afaict it doesn't mark any frames dirty.

My bad, I misread the code.

Comment 9

[fantasai]

Fix + reftest checked in.

Comment 10

[netwotkstudent]

I have same problem on FF16 too.
A box which is fixed is not showing on first page but in second and .... pages.
seems not fixed 100%