84017 - [PATCH]grey page prints out after printing this page with filled in form fields

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[sujay]

using 6/1 build of netscape

1) launch netscape
2) load above URL
3) fill out the form fields

do not click any Form submit buttons.

4) File | Print

look at the output. its an entire page of just a grey background.

Comment 1

[sujay]

oops wrong person...

Comment 2

[sujay]

actually it happens with any page with forms, not necessarily only
with the one supplied in this bug report.

Comment 3

[dcone (gone)]

*** Bug 84090 has been marked as a duplicate of this bug. ***

Comment 4

[dcone (gone)]

*** Bug 83661 has been marked as a duplicate of this bug. ***

Comment 5

[Kevin McCluskey (gone)]

It only crashes with a release build. Debug builds do not crash. My guess is
that we have pointer which has not been initialized to null.

Comment 6

[Kevin McCluskey (gone)]

Oops wrong bug. Ignore my previous comment

Comment 7

[dcone (gone)]

Attachment: this will get rid of the gray rectangle

Comment 8

[dcone (gone)]

This above patch gets rid of the gray rectangle.  The field is blank which is 
another bug, but the gray rectangle is gone.

Comment 9

[Kevin McCluskey (gone)]

Should it be a style change reflow or should it be a resize reflow?  
Marc: what do you think?

Comment 10

[dcone (gone)]

Attachment: Patch without old patch stuff.. sorry

Comment 11

[Kevin McCluskey (gone)]

patch 40462 looks good. r=[e-mail]

Comment 12

[Marc Attinasi]

I think a StyleChange reflow is fine, but since it is neither a resize or a
stylechange, it seems somewhat arbitrary to me - as long as it works. CC'ing
waterson in case he has some feeling about this, but sr=attinasi because, well,
it works and I think it is cool to issue a stylechange reflow when printing.

Comment 13

[Chris Waterson]

This brings cannons and mosquitos to mind. Could you at least comment it, noting 
the bug number, and saying something along the lines of why this fixes the 
problem? (Do we even know why this fixes the problem?)

Comment 14

[dcone (gone)]

Just a note here..
EndObserving document no longer does a reflow.. which history needs.. or we 
get a blank page for text fields.  So I did a StyleChangeReflow which will put 
in the new history.. and fill in the document content.  If there is no reflow.. 
and you have new history.. those fields will be gray.  Checked into the trunk.

Comment 15

[dcone (gone)]

checked into .92 branch

Comment 16

[sujay]

verified on 7/9 branch and trunk.

marking verified.

Comment 17

[sujay]

*** Bug 90898 has been marked as a duplicate of this bug. ***

Comment 18

[sujay]

*** Bug 91274 has been marked as a duplicate of this bug. ***

Comment 19

[vidur (gone)]

Chris' comments about cannons and mosquitos were pretty valid. Another reflow to
get form state is pretty heavy-handed and, in fact, seems to have led to several
problems related to incremental layout. The main problem is that layout history
state isn't set correctly at the time of initial reflow. I'm attaching a patch
that avoids the second reflow.

Comment 20

[vidur (gone)]

Attachment: patch to avoid second resize reflow

Comment 21

[vidur (gone)]

The whole notion of layout history state isn't managed correctly. It should be
that such state is available anytime frames are created. In actuality, it's
rarely  (if ever) passed along to the frame constructor. I believe the only bit
of code that allows form state to be restored exists in
nsPresShell::ContentAppended. For loaded pages, most new content goes through
this method and state is correctly restored. For printed pages, the content
hierarchy already exists and frame creation happen as a result of a call to
nsPresShell::InitialReflow. My patch adds a layout history state restoration to
InitialReflow. 

A better fix (but one that would involve more code change) would move history
state restoration out of nsPresShell::ContentAppended. nsPresShell methods that
drive frame creation should all pass in history state through the style set to
the frame constructor. In most cases, the frame constructor methods already
accept a history state parameter, though null is currently passed in. The frame
constructor will correctly restore state on a per-frame basis.

Comment 22

[vidur (gone)]

Reopening bug since the previous fix caused other printing regressions.

Comment 23

[vidur (gone)]

Changing target milestone to 0.9.5 and reassigning to myself. Should this also
go on the Netscape 0.9.4-based branch?

Comment 24

[Marc Attinasi]

Vidur, that patch looks nice and simple. What kind of pages did you test it
with? I'm especially curious about frameset documents (and iframes) since
printing them can be, er, onerous.

Comment 25

[vidur (gone)]

Can't say I've done much testing yet. There doesn't seem to be a printing
regression test, though Kevin mentioned that Don used a combination of tests
based on the layout regression testing system and some of the viewer examples.
Assuming I do adequate testing, can I get reviews?

Comment 26

[Cindy Roberts]

When you have reviews, please mark as nsbranch+ so we can get this on the PDT
radar.  If it gets PDT+ed then you can check it into the branch.

Comment 27

[vidur (gone)]

Attachment: Added minor nsFrameManager change to deal with frames with not content.

Comment 28

[Chris Waterson]

Looks good to me. Do you want to propagate the rv from GetContent()? Or just
return NS_OK? r/sr=waterson

Comment 29

[Marc Attinasi]

Comment on attachment 50737 [details] [diff] [review]
Added minor nsFrameManager change to deal with frames with not content.

r=attinasi

Comment 30

[Nisheeth Ranjan]

Marking nsbranch+, now that reviews are done.

Comment 31

[karnaze (gone)]

*** Bug 101264 has been marked as a duplicate of this bug. ***

Comment 32

[karnaze (gone)]

Vidur, can you please remove the workaround to this bug that I added in the 
patch to bug 82401 (I wasn't sure if you were going to fix this in time for the 
m0.9.4 branch). Look for "bug 101264" in nsDocumentViewer.cpp and 
nsSimplePageSequence.cpp. There are 3 places that are well commented. Actually, 
if you don't fix them per the comments, then your patch probably won't work 
anyway.

Also, it may not be a bad idea to run some printing tests and regression tests. 
The table regression tests are located in tests/table/printing. I guess it is 
remotely possible that some printing code has become dependent on a resize 
reflow happening.

Comment 33

[Kevin McCluskey (gone)]

*** Bug 89236 has been marked as a duplicate of this bug. ***

Comment 34

[kinmoz]

Attachment: Patch v3 (vidur's last patch + backout of karnaze's workaround for bug 82401)

Comment 35

[kinmoz]

As suspected, I can still reproduce the crash seen in bug 89236 with Patch v3 in
my Mozilla 09/27/01 Win32 debug build. I'm going to un-dup 89236 from this bug.

Comment 36

[rods (gone)]

This patch fixes bug 101987 (which was a regression caused by Karnaze's 
checkin), but bug 101975 is still missing content. 

I had to use (instead of 4497):
http://www.mcsebraindumps.com/dump.php?bd_id=4499

And it was missing content on the third page (this may have always been there)

Comment 37

[Jaime Rodriguez, Jr.]

pls check it in - PDT+

Comment 38

[vidur (gone)]

Fix checked into the MOZILLA_0_9_4_BRANCH on 9/28 at 8:21pm.

Comment 39

[vidur (gone)]

Fix checked into the trunk on 9/29.

Comment 40

[sujay]

verified in 10/1 branch. looks good.

will verify on trunk later.

Comment 41

[karnaze (gone)]

*** Bug 101987 has been marked as a duplicate of this bug. ***

Comment 42

[sujay]

verified on 10/25 trunk also.