167775 - IFrames do not print in the correct location

Status: VERIFIED FIXED

(Core :: Printing: Output, defect)

Description

[rods (gone)]

When an iframe is on the second page it doesn't print.

Comment 1

[rods (gone)]

Attachment: patch

Fixes how IFrame locations are calculated

Comment 2

[kinmoz]

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;
	   }

Comment 3

[kinmoz]

I meant to say that the infinite loop will occur if |isParent| is true.

Comment 4

[kinmoz]

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. :-)

Comment 5

[rods (gone)]

Attachment: patch v2

Reworked the patch a little, doAddInParentsOffset is only needed when printnig
an IFrame PO.

Comment 6

[rods (gone)]

Attachment: patch v3 (v2 is wrong diff)

Comment 7

[dcone (gone)]

Comment on attachment 98747 [details] [diff] [review]
patch v3 (v2 is wrong diff)

r=dcone

Comment 8

[kinmoz]

Comment on attachment 98747 [details] [diff] [review]
patch v3 (v2 is wrong diff)

sr=[e-mail]

Just add the comment regarding aX and aY.

Comment 9

[rods (gone)]

fixed - you should see the contents of iframes when they print on the second,
3rd page etc.

Comment 10

[Gayatri Rimola]

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

Comment 11

[Daniel Veditz [:dveditz]]

Please get a branch patch and get this verified on the trunk before renominating
adt1.0.2

Comment 12

[sujay]

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.

Comment 13

[rods (gone)]

Attachment: patch for 1.0.2 branch

patch ported over (works great)

Comment 14

[kinmoz]

Comment on attachment 104898 [details] [diff] [review]
patch for 1.0.2 branch

sr=[e-mail]

Comment 15

[dcone (gone)]

Comment on attachment 104898 [details] [diff] [review]
patch for 1.0.2 branch

r=dcone

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 104898 [details] [diff] [review]
patch for 1.0.2 branch

a=[e-mail] for 1.0 branch; change mozilla1.0.2+ to fixed1.0.2 when
checked in

Comment 17

[rods (gone)]

fixed

Comment 18

[Michael Buckland]

Just talked to Rod.  This has landed on the branch.  Updating keywords and
linking  to our meta.

Comment 19

[sujay]

verified on 11/6 branch build.