109914 - Printing document resets form control values

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[rods (gone)]

Now when we print a document the values in the form controls get reset. Also, 
the form control frames for printing do not have the correct values.

So example, text fields have NO values when they print.

The problem is that the current routines for setting the presentation state do 
not distinguish between the primary PresShell and secondary PresShells and often 
end up setting the content or resetting the content to their initial values.

The code below is a fix for the CheckBox, but I include this as a suggestion as 
to how things need to be fixed. You may need to do some factoring of code:

Index: nsGfxCheckboxControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsGfxCheckboxControlFrame.cpp,v
retrieving revision 1.40
diff -u -r1.40 nsGfxCheckboxControlFrame.cpp
--- nsGfxCheckboxControlFrame.cpp	2001/11/03 06:23:17	1.40
+++ nsGfxCheckboxControlFrame.cpp	2001/11/13 18:12:42
@@ -53,6 +53,8 @@
 #include "nsIServiceManager.h"
 #include "nsIDOMNode.h"
 
+#include "nsIPresShell.h" // for PresState
+#include "nsIDocument.h"
 
 //------------------------------------------------------------
 nsresult
@@ -284,7 +286,22 @@
 nsGfxCheckboxControlFrame::InitializeControl(nsIPresContext* aPresContext)
 {
   nsFormControlFrame::InitializeControl(aPresContext);
-  nsFormControlHelper::Reset(this, aPresContext);
+
+  nsCOMPtr<nsIPresShell> presShell;
+  aPresContext->GetShell(getter_AddRefs(presShell));
+  if (!presShell) return;
+
+  nsCOMPtr<nsIDocument> doc;
+  presShell->GetDocument(getter_AddRefs(doc));
+  if (!doc) return;
+
+  nsCOMPtr<nsIPresShell> primaryPresShell;
+  doc->GetShellAt(0, getter_AddRefs(primaryPresShell));
+  if (!primaryPresShell) return;
+
+  if (presShell.get() == primaryPresShell.get()) {
+    nsFormControlHelper::Reset(this, aPresContext);
+  } 
 }
 
 //------------------------------------------------------------

Comment 1

[John Keiser (jkeiser)]

Print Preview and Print both work for me for input textfields and selects. 
Checkboxes and radio buttons do not, because they are being initialized in
frames, as you diagnosed.  Are you sure text fields are failing?  If they are,
it can't be because of the reason below.  I'll try a Windows box at some time today.

If it's just checkboxes and radio buttons, the problem below will be fixed when
we move their values into content.  If the below fix for checkbox works for
radio too I will try and get a patch today anyway; it may be a week or more
before these things are moved to content (especially radio).

Comment 2

[rods (gone)]

I have a fresh pull on windows and if the input type=text has a default value 
set in the source then that is the value that prints out, if I type in a new 
value I still get that default value.

Comment 3

[John Keiser (jkeiser)]

Ah, I see it now, it's happening for input type=text but not textarea. 
Investigating.

Comment 4

[John Keiser (jkeiser)]

Two more weird things.

1. This happens *sometimes* but not always.  When it does happen (and it happens
pretty often) hitting Print Preview a *second* time makes the values show up. 
Issue with timing?

2. When I hit Print Preview, Back, Forward, radio buttons are all deselected. 
If I enter comment in the bug and submit, I get "knob is not defined."  Radio
buttons are also deselected.

Comment 5

[John Keiser (jkeiser)]

Attachment: Textbox Form

This one fails if you type in value.  sicking honed right in on the problem,
and a fix (and explanation if it works) is forthcoming.

Comment 6

[John Keiser (jkeiser)]

Attachment: Textarea test

OK, the problem is:
1. Frame is created for HTML page.  Editor is created.	Frame owns value.
2. Frame is created for print preview / print.
3. During construction, new frame realizes that it does not own value and calls
content->GetValueInternal().
4. GetValueInternal() gets value from content instead of primary frame.

To fix it, I've completely rid us of GetValueInternal() / SetValueInternal() in
favor of GetValue() / SetValue().  To avoid recursion, we are checking whether
the frame owns the value in GetValue() and SetValue() instead of in the frame
like we were before (this necessitated adding a method to
nsIGfxTextControlFrame2).

Comment 7

[John Keiser (jkeiser)]

Attachment: The Actual Patch

Oh ... you wanted the *patch*?

Comment 8

[John Keiser (jkeiser)]

Comment on attachment 57730 [details]
Textarea test

Since it's up here anyway, may as well make use of it :)

Comment 9

[John Keiser (jkeiser)]

Can someone with a printer try that patch out with Print?  (Both textarea and
input type=text.)  I have verified that it works for Print Preview only, though
I think the issues are the same.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

you shouldn't need the
+      aValue.Truncate();
in nsHTMLInputElement::GetValue


add an |NS_ENSURE_ARG_POINTER(aOwnsValue);| in the beginning of
nsGfxTextControlFrame2::OwnsValue


It might be nicer if [GS]etValue was defined in nsITextControlElement. That way
you wouldn't have to explicitly QI to both <textarea> and <input>. Might cause
confusion (but not really a problem) since the names are the same in the nsIDOM
interfaces.. your call


Other then that, r=sicking

Comment 11

[John Keiser (jkeiser)]

Attachment: Updated Patch

Removed Truncate(), added NS_PRECONDITION instead of NS_ENSURE_ARG for
performance reasons, leaving [GS]etValue out of interface for anti-confusion
reasons.

Comment 12

[John Keiser (jkeiser)]

Comment on attachment 57734 [details] [diff] [review]
Updated Patch

r=sicking from IRC

Comment 13

[John Keiser (jkeiser)]

OK, I don't want to check this puppy in until someone tests with Print.  Then we
need sr=.  rods?  jst?

This does not fix radio and checkbox.  I will get to these tomorrow.

The problem does not appear to occur with <select>.  Is anyone seeing it with
select?

One really annoying thing with Print Preview is it seems to overwrite all
presentation state.  When you hit Reload--which is the only way to get out of
Print Preview that I can tell--everything is back to default.  Not sure why this
is.  I'll investigate after radio and checkbox are fixed.

Comment 14

[rods (gone)]

This shouldn't be marked fixed, it wasn't checked in yet

Comment 15

[John Keiser (jkeiser)]

Hmm, I must have left a radio button checked from testing, sorry.  Does the
patch fix textboxes for you with Print?

Comment 16

[John Keiser (jkeiser)]

Attachment: rods's radio and checkbox fix (orthogonal)

This patch applies rods's fix to checkbox and radio.  It works for Print but
not Print Preview.  I believe Print Preview is of lower severity than this,
however, and I believe that problem will be fixed when radio / checkbox move to
content.

This patch is orthogonal to the other patch to make it clear that that patch
has not changed.

Comment 17

[Johnny Stenback (:jst)]

Comment on attachment 57896 [details] [diff] [review]
rods's radio and checkbox fix (orthogonal)

sr=jst

Comment 18

[Johnny Stenback (:jst)]

Comment on attachment 57734 [details] [diff] [review]
Updated Patch

sr=jst

Comment 19

[rods (gone)]

I pulled a tree and tested the printing and it now works.

r=rods on attachment 57734 [details] [diff] [review]

and on attachment 57896 [details] [diff] [review] (this until they get moved into content)

Comment 20

[John Keiser (jkeiser)]

Checked in.  Leaving open but reducing severity to look into the "form control
reset on reload" issue.

Comment 21

[John Keiser (jkeiser)]

Setting dependent in case that fixes this.

Comment 22

[Kevin McCluskey (gone)]

Moving to Moz1.1

Comment 23

[John Keiser (jkeiser)]

This all appears to be hunky-dory now.  Back, Forward and Reload don't even
appear on Print Preview so the problem cannot occur.

Comment 24

[sujay]

verified.