73846 - Make printing OBJECT frame safer

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Peter Lubczynski]

Currently, we try to instantiate another plugin. This bad and probably causing a 
lot of print crashes. 

Before full-blown printing, we need to make them safer.

Comment 1

[Peter Lubczynski]

Attachment: patch to make plugins safer

Comment 2

[Peter Lubczynski]

Ok, Don showed me how printing works. Because a whole frame model was being 
created we were actually trying to create and display another plugin instance!!

My patch basically just checks to see if we have a nsIPrintContext instead of a 
nsIPresShell and if so, bail.

Adding [seeking reviews]

Comment 3

[av (gone)]

I would write a static method, something like 
static PRBool isPrintingContext(nsCOMPtr<nsIPresContext> aPresContext)
since you are using it more than once. And this check will likely be needed in 
nsObjectFrame::Paint()

ra=av

Comment 4

[Peter Lubczynski]

It's just a simple QI. A little much for a method?

And this patch does also address ::Paint(). It's larger because of fixing the 
formatting.

Comment 5

[Peter Lubczynski]

Chris, can I get a sr=?

Comment 6

[Chris Waterson]

Coupla questions/comments...

1. So it looks like sometimes <object> frames can hold images? (Not sure exactly
   how that works; <object src="foo.jpg"> maybe?) If that's the case, isn't it
   ok to print?

2. You might want to call out that this is a hack by adding ``XXX'' to the
   comments.

3. Be sure to file a bug to implement printing on plugins. I'm not sure how big
   of a regression (?) this is going to be considered. (Guess I'm presuming
   that this makes it so that no plugin could ever print?)

Comment 7

[av (gone)]

There already is a bug on plugin printing issues. And this bailing out is not a 
hack. Well, at least in nsObjectFrame::Paint, because as far as I understand, 
some code will be added inside this if statement to address printing plugins.
Good thought about imsges. Peter, if your code skip printing images with this 
check we should avoid it. The way to see if this is an image is to check for 
FirstChild because images in the object frame are child image frames. We should 
probably add some member with type of object we have, but looks like so far we 
only have two: image and plugin/applet, so first child check will serve our 
purposes.

Comment 8

[Peter Lubczynski]

1) Images: Not a problem. I will move the check in Reflow to below the image 
check. I think it's the very next thing we check for so those should go through. 
 but the plugin won't be created again. Patch attached soon.

2) This is not a hack. We should NOT be creating a new instance of all plugins 
on a page just for printing. This was probably making a lot of sites crash while 
printing. This is the first step along the way to making plugins print. This 
needs to be done first.

My next plan is in the nsIPrintContext check in Paint(), to get the currently 
started instance of the plugin from the PluginHost that goes with the current 
URL that goes with the ObjectFrame we are attempting to print (paint). Then, 
I'll set up a NPPrint struct and pass that to NPP_Print(NPP, NPPrint)

3) Bug 27478 is the rest of the printing issue.

Comment 9

[Peter Lubczynski]

Attachment: better patch to address comments

Comment 10

[Peter Lubczynski]

Chris, what do you think about this patch. It addresses your issues?

Comment 11

[Chris Waterson]

Coolio. sr=waterson

Comment 12

[av (gone)]

Assume owner's approval too, Peter.

Comment 13

[Peter Lubczynski]

Patch checked in marking FIXED.

new revision: 1.206; previous revision: 1.205

Comment 14

[shrirang khanzode]

rubberstamp verified!