[viewpoint] NPP_HANDLE_EVENT for WM_PAINT doesn't pass in the updateRect

VERIFIED FIXED

Status

()

Core
Plug-ins
P2
critical
VERIFIED FIXED
16 years ago
14 years ago

People

(Reporter: Ari Berger, Assigned: av (gone))

Tracking

Trunk
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

16 years ago
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.

Comment 1

16 years ago
Andrei, can you take a look at this?
Assignee: peterl → av
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

16 years ago
nom'ing
Keywords: edt0.9.4
(Assignee)

Comment 3

16 years ago
Is passing the rectangle documented anywhere?
(Reporter)

Comment 4

16 years ago
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).
(Assignee)

Comment 5

16 years ago
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?
(Reporter)

Comment 6

16 years ago
Correct- they passed (top, left, bottom,right).
(Assignee)

Comment 7

16 years ago
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?
(Assignee)

Comment 8

16 years ago
Peter, in this patch I basically reused on Windows what we already have on Mac 
for calculating the dirty rectanle, excluding FixUpWindow.

Comment 9

16 years ago
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....

Comment 10

16 years ago
Created attachment 62056 [details] [diff] [review]
patch resolving conflicts for 0.9.4 branch

...for those that only have that branch...
(Reporter)

Comment 11

16 years ago
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.
(Reporter)

Comment 12

16 years ago
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
(Assignee)

Comment 13

16 years ago
Thanks, Ari, I'll give it a try. One should also probably nuke xpti.dat and 
components.reg when switching to the new plugin.
(Assignee)

Comment 14

16 years ago
Comment on attachment 62056 [details] [diff] [review]
patch resolving conflicts for 0.9.4 branch

New patch is coming.
Attachment #62056 - Attachment is obsolete: true
(Assignee)

Comment 15

16 years ago
Comment on attachment 62041 [details] [diff] [review]
patch v.1

New patch is coming.
Attachment #62041 - Attachment is obsolete: true
(Assignee)

Comment 16

16 years ago
Hm... I mean all previous patches are now obsolete, I have a new patch which 
passes the correct dirty rectangle. Will pass it tomorrow.
(Assignee)

Updated

16 years ago
Summary: NPP_HANDLE_EVENT for WM_PAINT doesn't pass in the updateRect → [viewpoint] NPP_HANDLE_EVENT for WM_PAINT doesn't pass in the updateRect
(Assignee)

Comment 17

16 years ago
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.
(Reporter)

Comment 18

16 years ago
Preliminary testing looks great! I'll run it through a bunch more test cases, 
but it looks like this does the trick for me.
(Assignee)

Updated

16 years ago
Keywords: patch

Comment 19

16 years ago
Comment on attachment 62409 [details] [diff] [review]
patch v.2

r=peterl
Attachment #62409 - Flags: review+

Comment 20

16 years ago
Plusing for 094 based on Ari's feedback
Keywords: edt0.9.4 → edt0.9.4+

Comment 21

16 years ago
Comment on attachment 62409 [details] [diff] [review]
patch v.2

sr=attinasi
Attachment #62409 - Flags: superreview+
(Assignee)

Comment 22

16 years ago
Setting milestone to be on par with the two bugs this one depends on.
Severity: major → critical
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.7
(Assignee)

Comment 23

16 years ago
In the trunk. Nominating for 0.9.7.
Keywords: patch → mozilla0.9.7
(Assignee)

Comment 24

16 years ago
In 0.9.4, marking fixed.
Keywords: mozilla0.9.7
Target Milestone: mozilla0.9.7 → ---

Comment 25

16 years ago
reso/fxd. will verify asap.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 26

16 years ago
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). 
Status: RESOLVED → VERIFIED

Updated

16 years ago
Keywords: fixed0.9.4

Comment 27

16 years ago
adding keyword 'verified0.9.4'
Keywords: verified0.9.4

Updated

14 years ago
Keywords: fixed0.9.4
You need to log in before you can comment on or make changes to this bug.