From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0) BuildID: In Netscape 4.x, we received the updaterect for WM_PAINT messages in the lparam. Mozilla passes us NULL. This has all sorts of bad consequences, including performance problems, and (more seriously) drawing errors when part of the plugin is covered by another window. Reproducible: Always Steps to Reproduce: 1.You should be able to reproduce this with any content on any plugin- the code in nsObjectFrame.cpp always sets the lparam to NULL 2.To see the negative consequences is somewhat more involved- install our plugin (http://cole.viewpoint.com/~aberger/setwindowbug/viewpointplugin.zip) 3.View http://cole.viewpoint.com/~aberger/setwindowbug/index.html (note: viewing this content depends on having the patch from 114667) 4. Partially cover the plugin area with another window and then move it away 5. interact with the object- you should see that our image is stuck in the background Because we don't know which part of the window you redrew, we assume you redrew the whole thing. We assume that our image is gone, so we redraw our image. But you have not really redrawn the whole plugin window, just part of it. We need to know which part.
Andrei, can you take a look at this?
Is passing the rectangle documented anywhere?
I have not seen it in documentation. In fact, the Netscape 4.x API documentation just says that it will windows message, which doesn't pass it in. However, Netscape 4.x passed it in, and it is very important for us. If you have only redrawn part of the window (and didn't clear the rest of it), and we paint the whole window, you will see a sort of double-image- we are still drawn on the screen (from the last time we painted) and we will draw again on top of it. Normally when you get a WM_PAINT message, you can call GetUpdateRect. If I try that when you tell me to paint, I get back 0 (meaning there is no update rect).
Just to confirm. 4x passed the rectangle in the form of (top, left, bottom, right) rather than (x, y, width, height) and this is what we want, is that right?
Correct- they passed (top, left, bottom,right).
Created attachment 62041 [details] [diff] [review] patch v.1 Ari, I also assume that the rect struct is Windows style, i.e. all fields are of LONG type, rather than NPRect with uint16 fields. This patch passes whatever dirty rectangle we have in nsPluginInstanceOwner::Paint assuming it is correct. I still some painting problems but not sure what they are related to. This patch also includes "alternative patch" from bug 114667 (http://bugzilla.mozilla.org/showattachment.cgi?attach_id=61428) Peter, Patrick, please have a look. Ari, can you please test it?
Peter, in this patch I basically reused on Windows what we already have on Mac for calculating the dirty rectanle, excluding FixUpWindow.
That sounds correct because windowless plugins events work similar to Mac. I'd like to try this out, but I'm a bit backlogged on testing patches....
Created attachment 62056 [details] [diff] [review] patch resolving conflicts for 0.9.4 branch ...for those that only have that branch...
Andrei- I tried the patch out. It is part of the way there, but not all the way. The rect that I am getting seems to be the correct size (at least, at looks reasonable) but it is offset vertically. For example, on the piece of content I was viewing (i will try it on the simplified content that i reference in this bug tomorrow), I get the following results: On the first paint call (when I would expect to be told to paint the entire window, I get: My entire window is (462, 45) -(818,345) (or height, width = 300,356) the rect you pass me is (462,-240) -(818,60) (or height, width = 300,356) As I move around a window in front of us to see what we get told to paint, the bottom coordinate stays at 60, the top changes. So again, looks like correct dimensions, wrong origin. I hope this helps. Tomorrow we are doing a new build of our plugin which I can make available, and I will try this against the test content that I have posted.
If you want to try to reproduce the problem that I am seeing, I put up a new version of our plugin. If you listen with a debugger, I send out debugstrings with the rect that I get in setwindow and the rects that I get from you. Same content URL: http://cole.viewpoint.com/~aberger/setwindowbug/index.html If you already have our plugin installed, you can just replace the dll and xpt from: http://cole.viewpoint.com/~aberger/setwindowbug/npViewpoint.dll http://cole.viewpoint.com/~aberger/setwindowbug/npViewpoint.xpt (note that we renamed our dll- you should probably delete the old one). If you haven't installed our plugin, you also need to run the executable: http://cole.viewpoint.com/~aberger/setwindowbug/VMPFullInstall_3_0_8_201.exe
Thanks, Ari, I'll give it a try. One should also probably nuke xpti.dat and components.reg when switching to the new plugin.
Comment on attachment 62056 [details] [diff] [review] patch resolving conflicts for 0.9.4 branch New patch is coming.
Comment on attachment 62041 [details] [diff] [review] patch v.1 New patch is coming.
Hm... I mean all previous patches are now obsolete, I have a new patch which passes the correct dirty rectangle. Will pass it tomorrow.
Created attachment 62409 [details] [diff] [review] patch v.2 This patch presumably calculates the dirty rectangle correctly. It also assumes that fixes for bug 116093 and bug 116108 are already applied. Please try and review.
Preliminary testing looks great! I'll run it through a bunch more test cases, but it looks like this does the trick for me.
Comment on attachment 62409 [details] [diff] [review] patch v.2 r=peterl
Plusing for 094 based on Ari's feedback
Comment on attachment 62409 [details] [diff] [review] patch v.2 sr=attinasi
Setting milestone to be on par with the two bugs this one depends on.
In the trunk. Nominating for 0.9.7.
In 0.9.4, marking fixed.
reso/fxd. will verify asap.
verif on 1226 trunk and 1225 branch builds that this works. Arun clarified that changing the useragent string for commercial browser might get this testcase to work (which is correct).
adding keyword 'verified0.9.4'