Closed Bug 167775 Opened 21 years ago Closed 21 years ago

IFrames do not print in the correct location

Categories

(Core :: Printing: Output, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.2beta

People

(Reporter: rods, Assigned: rods)

References

Details

Attachments

(2 files, 2 obsolete files)

When an iframe is on the second page it doesn't print.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2beta
Attached patch patch (obsolete) — Splinter Review
Fixes how IFrame locations are calculated
Depends on: 129034
Comment on attachment 98609 [details] [diff] [review]
patch

-- Should aX and aY be initialized just in case some null
   pointer forces this method to exit before the while loop
   is executed? If you are assuming aX and aY are initialized
   prior to the call, please add a comment that states that.


+static void GetIFramePosition(nsPrintObject * aPO, nscoord& aX, nscoord& aY)
+{
+  if (aPO->mParent != nsnull) {


-- That's a long pointer chain to follow. :-) Does the caller ensure
   that none of those pointers can ever be null? Also, we are asserting
   if we have a null frameMan, but we'll use it anyways, do we want
   to add a null check that causes an early return before a null pointer
   is used?


+    aPO->mParent->mPresShell->GetFrameManager(getter_AddRefs(frameMan));
+    NS_ASSERTION(frameMan, "No Frame manager!");


-- "we we out wat"? :-)


+	 // For IFrames, we locate the "inner" frame in the Parent document
+	 // then start calculating the location as we we our wat out to the 
+	 // the pageContentFrame


-- Won't this be an infinite loop if |isParent && doAddInParentsOffset| is
true?
   That is, |po = po->mParent| will yield the same pointer right? Also, is it
   possible to run into that situation in the iframe loop above this code?


+	   nsPrintObject * po = aPO;
+	   while (po != nsnull) {
+	     PRBool isParent = po == aPO->mParent;
+	     if (!isParent || (isParent && doAddInParentsOffset)) {
+	       x += po->mRect.x;
+	       y += po->mRect.y;
+	     }
+	     po = po->mParent;
	   }
I meant to say that the infinite loop will occur if |isParent| is true.
Ok duh, for some reason when I looked at:

+            PRBool isParent = po == aPO->mParent;

I was thinking

+            PRBool isParent = po == pO->mParent;

which would've been a problem ... so I guess it's not. :-) Thanks for setting me
straight rod. :-)
Attached patch patch v2 (obsolete) — Splinter Review
Reworked the patch a little, doAddInParentsOffset is only needed when printnig
an IFrame PO.
Attachment #98609 - Attachment is obsolete: true
Attachment #98746 - Attachment is obsolete: true
Comment on attachment 98747 [details] [diff] [review]
patch v3 (v2 is wrong diff)

r=dcone
Attachment #98747 - Flags: review+
Comment on attachment 98747 [details] [diff] [review]
patch v3 (v2 is wrong diff)

sr=kin@netscape.com

Just add the comment regarding aX and aY.
Attachment #98747 - Flags: review+ → superreview+
fixed - you should see the contents of iframes when they print on the second,
3rd page etc.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Keywords: nsbeta1+
Keywords: adt1.0.2
Sujay, 
Can you please verify this on the trunk. It will help us determine whether it
should be included in the Blackbird or not. 
Thanks, Gayatri
Please get a branch patch and get this verified on the trunk before renominating
adt1.0.2
Keywords: adt1.0.2adt1.0.2-
verified in 9/26 trunk build.

I printed out http://www.news.com and it seemed like it printed
out correctly..Verified with layout QA engineer also.
Status: RESOLVED → VERIFIED
patch ported over (works great)
Comment on attachment 104898 [details] [diff] [review]
patch for 1.0.2 branch

sr=kin@netscape.com
Attachment #104898 - Flags: superreview+
Comment on attachment 104898 [details] [diff] [review]
patch for 1.0.2 branch

r=dcone
Attachment #104898 - Flags: review+
Keywords: adt1.0.2-adt1.0.2
Comment on attachment 104898 [details] [diff] [review]
patch for 1.0.2 branch

a=rjesup@wgate.com for 1.0 branch; change mozilla1.0.2+ to fixed1.0.2 when
checked in
Attachment #104898 - Flags: approval+
fixed
Keywords: fixed1.0.2
Just talked to Rod.  This has landed on the branch.  Updating keywords and
linking  to our meta.
Depends on: blackbird
Keywords: adt1.0.2adt1.0.2+
verified on 11/6 branch build.
You need to log in before you can comment on or make changes to this bug.