225832 - plugin printing in wrong location

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[~Happy Dude~]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20031007 Firebird/0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20031007 Firebird/0.7

When this entire page is printed while viewing JPEG2000 content on it with the
linked JPEG2000 plugin for Mozilla, the JPEG2000 imagery is printed in the wrong
location.

As the developer of this plugin, I can tell you that the problem seems to be
that the NPWindow structure passed (within a myriad of parent structures/unions)
to the NPP_Print function has x,y coordinates that do not take into account the
fact that the plugin is nested within other elements on the html page.

For example, on the example page, the plugin is initiated using an <EMBED> tag
that is contained within a <DIV> tag.  For on screen display purposes, this has
worked fine, but when it comes time to print, the plugin is printed as though
the <EMBED> tag is not nested at all.

Reproducible: Always

Steps to Reproduce:
1.View http://www.linfar-tech.com/geoserver_demo/
2.Select the 'JP2' option from the 'Display Format' combobox
3.Use the Mozilla File|Print menu sequence to print the entire page.

Actual Results:  
The page is printed, but the imagery displayed in the plugin is located at the
top-left corner of the page and clipped by overlapping elements.

Expected Results:  
The imagery displayed in the plugin should be printed at the same offset on the
page as it exists on the screen.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Hmm... Do you see the problem only when the <div> is absolutely positioned? 
That is, if you just have a page with no positioning in it and a <div> in that
page (below some text, say) and you put the <embed> inside that <div>, does the
problem appear?

Comment 2

[Boris Zbarsky [:bzbarsky]]

To be precise, the origin of the window, for printing is set by
GetOffsetFromView() in nsObjectFrame::Paint().  Then this is used with the
rendering context that got passed to Paint().  roc, what coordinate system is
that rendering context in? Is it in the coord system of the nearest widget which
contains the frame?  And what happens if the <object> itself has a view or widget?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Paint() is called with the rendering context translation setup so (0,0)
corresponds to the top-left corner of the frame --- pretty much what you'd
expect (for a change :-) ). See
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsContainerFrame.cpp#282

I don't know why GetOffsetFromView is being called there at all.
GetOffsetFromView returns the offset from this frame's origin to the origin of
the nearest enclosing frame with a view NOT including this frame. So here it
will be returning the offset to whatever frame with a view encloses this
nsObjectFrame. Which is kinda nuts.

But that doesn't really explain this bug since (without having looked at the
testcase myself) this particular bug should render the plugin further down and
to the right than expected, while this bug has the plugin rendering at the top left.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Paint() is called with the rendering context translation setup so (0,0)
> corresponds to the top-left corner of the frame

That doesn't matter.  The way plugin painting works on Windows when printing is
that we just give the plugin the native DC for our rendering context.  See
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsObjectFrame.cpp#1758

We then proceed to tell the plugin where within that native DC it lives by
setting the "window" rect using that GetOffsetFromView business.

Compare this to non-print mode (for windowless plugins, at least), where we set
the "window" rect using the aRenderingContext offset (again on windows).  Then
what you said kicks in.  See
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsObjectFrame.cpp#1825

Summary:  All this code is just totally broken.  It's amazing it ever works.

Fixing it even worse, since the print version of the code is all tangled up
between the Unix, Mac, and Windows versions... Unless they're all buggy in
exactly the same way?

Comment 5

[~Happy Dude~]

Hey guys.  Well, I'm impressed with the quick responses.  I've just finished 
from some running around I had to do, and I've updated the plugin offered on 
the testcase page to include the latest changes I have made in an attempt to 
get embedded printing to work within mozilla.  So if you've gotten the plugin 
earlier, you should probably get it again.

Anyways, Boris, in response to comment #1, I don't know if an absolutely 
positioned DIV would still cause the same behaviour.  I will test on monday at 
work and let you know.

To give you guys more information, the plugin I've written is WINDOWED (NOT 
WINDOWLESS) and written for MSWindows.  Within the NPP_Print function, I'm 
given an HDC (provided by the browser) and a region (as defined within an 
NPWindow structure).  What I expected to happen would be that I draw to that 
HDC within that rect, and everything would turn out honky dory.  The problem is 
that the rect given to me is not correct.  It's positioned at the top left of 
the page (offset by the margins).

"It's amazing it ever works"?  That's hardly re-assuring :).

As a side note, when I first started this plugin, I went with a windowless 
approach (seems to integrate with the browser more nicely -- clips to the other 
elements around it etc...).  The way I switch between JPG and JP2 output is by 
dynamically deleting and replacing the portion of the page that contains the 
imagery.  When JP2 output is selected, I would delete the IMG tag that 
contained the JPG.  When JPG output is selected, I would delete the EMBED tag 
that contained the JP2.  However, I had to abandon this approach because I 
discovered that deleting a DOM node that contains a windowless plugin caused 
the browser to crash.  I thought it might be something wrong with my plugin 
code, but I managed to get the crash to happen even if the plugin being put in 
the form was one of the simple test plugins from the gecko sdk.  Anyways, 
that's an aside... I'm gonna post a seperate bug report for that one.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I don't expect you have a Linux version of your plugin hanging around, do you?  ;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Note that at the moment I can't get plug-ins to print at all in Linux, so I'm
sort of unable to test patches, even....  reporter, do you have the ability to
compile Mozilla and test things?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Something like this?

In typical fashion, I cannot locate a Linux plugin that actually prints.  Java
(1.4.2) has the decency to tell me "Printing is not yet implemented".  Flash
and RealPlayer, I have no idea why they don't print.  I'll work on locating a
plugin I can actually print so I can maybe test this for sanity...

Comment 9

[~Happy Dude~]

Boris,

  Ya, sorry to say this plugin is MSWindows only for now.  Sorry about that.

  As for building mozilla, I had a look at the build requirements, and I should 
be able to do it from my work machine on monday.  I've got all the required 
tools for windows(cygwin, msvc6 sp5, etc...), but I'm sure there will some 
hitches along the way, but it will get done.  I'm guessing you want me to try 
building the latest code from the main trunk of CVS, right?  Have attempted 
fixes been checked into there already?

Comment 10

[Boris Zbarsky [:bzbarsky]]

No, the fix can't be checked in until it has been reviewed, and it needs to be
tested first. ;) Hence my attempts to find a plugin that prints and my question
whether you can build Mozilla (and then apply the fix and rebuild to see what
effect it has).

Comment 11

[~Happy Dude~]

Attachment: demonstration of print problem using a 'print-to-postscript-file' method

Printed the testcase page using a postscript print-to-file print driver, then
converted the postscript file to a png to reduce file size.  Note, the image of
the plugin is being printed, but it seems to be positioned at the top left of
the page (offset by page margins).

Comment 12

[~Happy Dude~]

Just more information that has sprung to mind:

During my debugging of the plugin, I noted that the x,y coordinates passed to 
the plugin in the NPWindow structure were:

x: 300
y: 300

From the documentation I've read, these coordinates are in units of TWIPS.  
This seemed wrong to me right away, because the plugin SHOULD be positioned 
further down the page then it is positioned away from the left edge.

Comment 13

[~Happy Dude~]

oh ya, testing :).

Alright, I'll get the latest version from the main trunk of CVS on monday, 
build that, and then apply your patch attached here and see if that works.  
That will let me know whether your patch works or not, but I'm guessing you 
need mozilla developers to verify that the patch works and run the test suite 
to search for un-intended consequences of the patch.  Surely some of the 
developers use windows, right? :)

Comment 14

[Boris Zbarsky [:bzbarsky]]

> but I'm guessing you need mozilla developers to verify that the patch works

Not particularly...  I need someone to verify that it works and then Mozilla
developers to verify that it theoretically _should_ work.  ;)

> and run the test suite

Heh.  We have no useful printing tests, as far as I know.

And yes, some people do use Windows.  Good point.  Neil, have you got spare
cycles to test this?

Comment 15

[[e-mail]]

Only if the plug-in will work with a MingW build (I don't have MSVC).

Comment 16

[~Happy Dude~]

Attachment: New Proposed Patch

Boris,

I tried your last patch and it didn't work for me.  A quick inspection of the
changes showed that you were skipping the

aPresContext->GetTwipsToPixels(&t2p);

call in your patched version of the code.  This seems to be a mistake since the
t2p structure is required later on the lines that do:

    window.width = NSToCoordRound(mRect.width * t2p);
    window.height= NSToCoordRound(mRect.height * t2p);

So I just added the call to GetTwipsToPixels, and everything seems to be
working fine.  Printing WINDOWED (NOT WINDOWLESS) plugins in embedded printing
mode, under MSWindows SEEMS to work now.

I'll post another print-to-file example demonstrating the output.

I have not tested to see whether these changes negatively affect the printing
of WINDOWLESS plugins.

Also, please note that your previous patch seems to have been applied to a
different state of nsObjectFrame.cpp than the one I checked out on Monday
morning, 2003, as the line numbers were different.

Comment 17

[~Happy Dude~]

Attachment: Demonstration of fixed plugin printing

Comment 18

[Boris Zbarsky [:bzbarsky]]

Ah, yes.  Removing the GetTwipsToPixels() call there was a mistake...  ;)  And
yes, my version of the file was a revision or two behind; no substantive changes
happened in those revisions.

Good to know that this patch works.   Can you possibly point me to where we have
the plugin printing stuff documented, by the way?  I just want to make sure the
documentation and the code end up consistent and all...

Comment 19

[~Happy Dude~]

Hey Boris,

  Well, the printing documentation for plugins that I used would be found here:

http://devedge.netscape.com/library/manuals/2002/plugin/1.0/drawevent.html#99969
1

but I'm not sure that it needs updating.  Maybe just some fleshing out of the 
information.

Thanks again for your help, I'm quite impressed with the turn-around time in 
response to this bug.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 135742 [details] [diff] [review]
New Proposed Patch

Hrmph.	So we pass the origin in twips and the height/width in pixels?	That
makes soooooo much sense....

Anyway, I believe this is the way to go.  If someone could also test with the
Flash or Java plugins on windows, that would be much appreciated.  Without this
patch, a plugin in an absolutely positioned div should print at the wrong
position, I think

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 135742 [details] [diff] [review]
New Proposed Patch

Looks correct but this must be tested with Flash at least. I have a feeling
that people may have tried to work around this bug in some scary way.

Comment 22

[~Happy Dude~]

Boris,

  Ya, you're right, it seems very strange that the origin is passed in twips, 
but the dimensions are passed in pixels...  why does that end up working?

  Furthermore, it seems strange to me that the code doesn't seem to be taking 
the margins into account explicitly like it was before.  Maybe that's all 
accounted for in the aRenderingContext.GetCurrentTransform(...) call.  I dunno, 
this codebase is all foreign to me.

Comment 23

[Boris Zbarsky [:bzbarsky]]

Well, as I pointed out Flash on Linux no printee.  So someone on Windows needs
to test this...  If we want this by 1.6, it probably needs testing today.

Comment 24

[~Happy Dude~]

I'll see about testing flash printing when I get to work.

Comment 25

[Boris Zbarsky [:bzbarsky]]

See comment 20 for what you're looking for.  Plugins in abs pos divs should
print in the wrong location without this patch and in the right location with it....

Comment 26

[~Happy Dude~]

Attachment: test using flash plugin on the same page as testcase (PRE FIX APPLICATION)

Because I had trouble finding another page that has an absolutely positioned
DIV with a plugin in it, I just made a test page that is the same as the
existing testcase, but has a flash plugin embedded in it instead of our plugin.


Notice the printing is incorrect as it was with our JP2 plugin.

Comment 27

[~Happy Dude~]

Attachment: test using flash plugin on the same page as testcase (AFTER FIX APPLICATION)

Now, notice the printing has been corrected.

For the curious, the flash file shown in this test is:

http://www.studentlife.uoguelph.ca/startonline/swf/weekly%20themes/oweek.swf

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Looks great. Check this sucker in

Comment 29

[Boris Zbarsky [:bzbarsky]]

Taking.  I'll check it in tonight.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Checked in.