Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Visible Region Notify message does not work when scrolling client window

RESOLVED FIXED

Status

()

Core
Plug-ins
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: KO Myung-Hun, Assigned: Peter Weilbacher)

Tracking

({fixed1.8.1.15})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.8.1.12) Gecko/20080202 SeaMonkey/1.1.8
Build Identifier: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.8.1.12) Gecko/20080202 SeaMonkey/1.1.8

When scrolling client window including plugin window, Visible Region Notify messages is not delivered to plugin window correctly. That is, in normal case the messages are delivered to plugin window in following order.

  WM_VRNDISABLED
  WM_MOVE
  WM_VRNENABLED

But, in case of plugin window, WM_MOVE is not wrapped by WM_VRNDISABLED and WM_VRNENABLED pair.

Or, is there a way to know when plugin window is about to be moved or client window try to scroll its area ?

Reproducible: Always

Steps to Reproduce:
1. Open html including plugin
2. Scroll up and down
3. 
Actual Results:  
WM_MOVE
WM_VRNDISABLED
WM_VRNENABLED

Expected Results:  
WM_VRNDISABLED
WM_MOVE
WM_VRNENABLED
(Assignee)

Comment 1

10 years ago
Confirming that there is a problem. (I still haven't debugged this to see the messages, but I have seen that the plugin content smears onto the page outside the plugin area when scrolling.

My ignorance of PM development shows again. I looked up the WM_VRN* messages in the toolkit documentation where it is said that WM_VRNDISABLED happens when windows are resized and/or after a WinLockWindowUpdate() call. We don't have that call in our sources and we don't generate either of the WM_VRN* messages. We actually don't even handle or generate WM_MOVE, either. Should we?

Mike, you probably at some point understood the message processing. Do you have any ideas?
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

10 years ago
By chance I found bug 387701 which seems related to this problem. Maybe we can do something similar to the Windows patch in attachment 285811 [details] [diff] [review] to fix this problem?
(Assignee)

Comment 3

10 years ago
Created attachment 306630 [details] [diff] [review]
first try, doesn't work

OK, so this is the Windows patch adapted for OS/2. Unfortunately, it doesn't work as the MPlayer plugin window also returns a class name of "MozillaWindowClass". And I don't know what else could be used to detect if a window is ours or a plugins'.

But even if I remove the classname check and just repaint all child windows it still spills the movie over onto the surrounding page when scrolling. So back to the start...
(Assignee)

Comment 4

10 years ago
(In reply to comment #3)
> work as the MPlayer plugin window also returns a class name of
> "MozillaWindowClass".

Stupid me, WinBeginEnumWindows only enumerates the direct children and the MPlayer window is two levels further down. Because when I do it recursively I get
   classname(child=0x8000021a)=MozillaWindowClass
   classname(child=0x80000258)=#1
   classname(child=0x8000025a)=WC_MPLAYER
But it still doesn't work. Just forcing a repaint of the whole window works but is of course horribly slow. We probably need to find a way to repaint the Mozilla window at the position where the plugin was before the scroll. That could get more complicated...

Could the reason for this bug be our use of SW_INVALIDATERGN? While browsing the toolkit docs I found this about it:
   The invalid region created as a result of the scroll is added to the
   update regions of those windows affected. This may result in sending
   WM_PAINT messages to CS_SYNCPAINT windows before the call returns.
The second sentence could mean some out-of-order messages as described in comment 0...
(Reporter)

Comment 5

10 years ago
Hi/2.

How about WinBroadcastMsg() as the following

    WinBroadcastMsg( hwndPlugin, WM_VRNDISABLED, 0, 0, BMSG_SEND ) 

before about to scroll ?

Here, hwndPlugin is the window handle of plugin(MozillaWindowClass) not MPlayer(WC_MPLAYER).

KO Myung-Hun

(In reply to comment #4)
> (In reply to comment #3)
> > work as the MPlayer plugin window also returns a class name of
> > "MozillaWindowClass".
> 
> Stupid me, WinBeginEnumWindows only enumerates the direct children and the
> MPlayer window is two levels further down. Because when I do it recursively I
> get
>    classname(child=0x8000021a)=MozillaWindowClass
>    classname(child=0x80000258)=#1
>    classname(child=0x8000025a)=WC_MPLAYER
> But it still doesn't work. Just forcing a repaint of the whole window works but
> is of course horribly slow. We probably need to find a way to repaint the
> Mozilla window at the position where the plugin was before the scroll. That
> could get more complicated...
> 
> Could the reason for this bug be our use of SW_INVALIDATERGN? While browsing
> the toolkit docs I found this about it:
>    The invalid region created as a result of the scroll is added to the
>    update regions of those windows affected. This may result in sending
>    WM_PAINT messages to CS_SYNCPAINT windows before the call returns.
> The second sentence could mean some out-of-order messages as described in
> comment 0...
> 

(Assignee)

Comment 6

10 years ago
(In reply to comment #5)
> How about WinBroadcastMsg() as the following
> 
>     WinBroadcastMsg( hwndPlugin, WM_VRNDISABLED, 0, 0, BMSG_SEND ) 
> 
> before about to scroll ?

I had thought about something like that but didn't actually try. Wouldn't you get two WM_VRNDISABLED messages, one before WM_MOVE, one afterwards? What effects do you think this would have?

> Here, hwndPlugin is the window handle of plugin(MozillaWindowClass) not
> MPlayer(WC_MPLAYER).

So, I would need to find the WC_MPLAYER window and then twice step upwards in the window tree to find the related MozillaWindowClass window? That sounds like an ugly solution but I can give it a try.
(Reporter)

Comment 7

10 years ago
Hi/2.

(In reply to comment #6)
> (In reply to comment #5)
> > How about WinBroadcastMsg() as the following
> > 
> >     WinBroadcastMsg( hwndPlugin, WM_VRNDISABLED, 0, 0, BMSG_SEND ) 
> > 
> > before about to scroll ?
> 
> I had thought about something like that but didn't actually try. Wouldn't you
> get two WM_VRNDISABLED messages, one before WM_MOVE, one afterwards? What
> effects do you think this would have?
> 

Normally, WM_VRNDISABELD before WM_MOVE and WM_VRNENABLED after WM_MOVE.

But currently, this expect is not fulfilled. So we need to generate WM_VRNDISABLED message on purpose before scrolling plugin window.

Currently, MPlayer respond to WM_MOVE as well as WM_VRNDISABELD/WM_VRNENABLED pair as a workaround. So WM_DISABLED disable the output of MPlayer, and WM_MOVE enable it, again. In this case, smearing the outside of plugin window can be prevented. That is, WM_MOVE replace WM_VRNENABLED.

> > Here, hwndPlugin is the window handle of plugin(MozillaWindowClass) not
> > MPlayer(WC_MPLAYER).
> 
> So, I would need to find the WC_MPLAYER window and then twice step upwards in
> the window tree to find the related MozillaWindowClass window? That sounds like
> an ugly solution but I can give it a try.
> 

Huh ? Mozilla does not have a method to find plugin window, that is, child window directly ? The reason I suggested to use hwndPlugin is I thought Mozilla could know the plugin handle. If so, there is no need to enumerate windows in order to find MPlayer window.

Anyway, it is no matter the receiving window is the plugin window or MPlayer window. In either case, WM_VRNDISABLED is passed to MPlayer window.

Finally, invalidating the plugin window and the foreign window is irrelevant to this problem.

KO Myung-Hun
(Assignee)

Comment 8

10 years ago
(In reply to comment #7)
> Normally, WM_VRNDISABELD before WM_MOVE and WM_VRNENABLED after WM_MOVE.
> 
> But currently, this expect is not fulfilled. So we need to generate
> WM_VRNDISABLED message on purpose before scrolling plugin window.

I understand. But as the WM_VRNDISABLED that you currently get _after_ the WM_MOVE is not generated by us/Mozilla, you will then get two. The one we generate on purpose before the scroll and the one that is automatically generated somewhere else, after the scroll. I was just wondering if that was a problem for your code or not.


> Huh ? Mozilla does not have a method to find plugin window, that is, child
> window directly ?

Could be that it knows this somewhere in the plugin code which I don't know at all. In the nsWindow class where I started debugging (it is the main class to deal with the PM messages) it does not know that.

> Finally, invalidating the plugin window and the foreign window is irrelevant to
> this problem.

Not entirely irrelevant, as a forced invalidation of the whole content window after a scroll fixes the visible problem (no smearing of movie content on the webpage outside the plugin area any more). I don't know if it affects the WM_VRN* messages or not.
(Reporter)

Comment 9

10 years ago
Created attachment 307230 [details]
A test build of npmp.dll

A test build of npmp.dll to check VRN messages
(Reporter)

Comment 10

10 years ago
Hi/2.

(In reply to comment #8)
> (In reply to comment #7)
> > Normally, WM_VRNDISABELD before WM_MOVE and WM_VRNENABLED after WM_MOVE.
> > 
> > But currently, this expect is not fulfilled. So we need to generate
> > WM_VRNDISABLED message on purpose before scrolling plugin window.
> 
> I understand. But as the WM_VRNDISABLED that you currently get _after_ the
> WM_MOVE is not generated by us/Mozilla, you will then get two. 

I know.

> The one we
> generate on purpose before the scroll and the one that is automatically
> generated somewhere else, after the scroll. 

No. WM_VRNDISABLED is not generated whenever WM_MOVE, but after a bunch of WM_MOVE.

> I was just wondering if that was a
> problem for your code or not.
> 

No problem.

> 
> > Huh ? Mozilla does not have a method to find plugin window, that is, child
> > window directly ?
> 
> Could be that it knows this somewhere in the plugin code which I don't know at
> all. In the nsWindow class where I started debugging (it is the main class to
> deal with the PM messages) it does not know that.
> 

Ok.

> > Finally, invalidating the plugin window and the foreign window is irrelevant to
> > this problem.
> 
> Not entirely irrelevant, as a forced invalidation of the whole content window
> after a scroll fixes the visible problem (no smearing of movie content on the
> webpage outside the plugin area any more). I don't know if it affects the
> WM_VRN* messages or not.
> 

I think, this is a timing problem. The content window would repaint after the outside of the plugin window is smeared as the following.

    Blitting
    Scrolling
    Blitting cause to smear
    WM_MOVE delivered to plugin window and its child windows
    Repainting cause to clear

This is just my guess.

But you can confirm whether VRN messages wrap around WM_MOVE or not by using the attachment. I'm sorry, however, that the zip attachment cannot be downloaded as its original name. You should rename it to npmp.zip.

KO Myung-Hun


(Assignee)

Comment 11

10 years ago
Created attachment 307317 [details] [diff] [review]
simple fix as suggested by KO Myung-Hun

For some reason the debug plugin DLL doesn't output anything else than the others already did.

But anyway, your suggestion with the broadcast as in this patch seems to work well enough.

I have uploaded a test DLL to
http://sourceforge.net/project/showfiles.php?group_id=183811&package_id=213528&release_id=580725
The patched file is in the sm200802291049_wdgtos2.zip package and it should be copied over the file of the same name from the seamonkey-2.0a1pre.en-US.os2.2008022910.zip nightly build.
Assignee: nobody → mozilla
Attachment #306630 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Reporter)

Comment 12

10 years ago
Hi/2.

(In reply to comment #11)
> Created an attachment (id=307317) [details]
> simple fix as suggested by KO Myung-Hun
> 
> For some reason the debug plugin DLL doesn't output anything else than the
> others already did.
> 

There is no WM_MOVE, even ?

> But anyway, your suggestion with the broadcast as in this patch seems to work
> well enough.
> 
> I have uploaded a test DLL to
> http://sourceforge.net/project/showfiles.php?group_id=183811&package_id=213528&release_id=580725
> The patched file is in the sm200802291049_wdgtos2.zip package and it should be
> copied over the file of the same name from the
> seamonkey-2.0a1pre.en-US.os2.2008022910.zip nightly build.
> 

Strange. It does not work here as expect, still cause to smear.

Of course, WM_VRNDISABLED is passed before WM_MOVE.

Surely, does not corruption happen to you any more ?

I scrolled the content window by keyboard or mouse wheel.

KO Myung-Hun
(Assignee)

Comment 13

10 years ago
No, I don't see any WM_* messages, not even WM_MOVE. I do see them in the binary, though...

Too bad that it doesn't work for you. I also tried several different ways to scroll. I will try to create a DLL with the idea I had about invalidation. Or perhaps I should try one with WinLockWindowUpdate().
(Assignee)

Comment 14

10 years ago
A difference with our setups might be the smooth scrolling preference (Edit -> Preferences -> Content -> Use smooth scrolling in a current SeaMonkey trunk build). When that is switched off the problem is more visible and I indeed then see a bit of the movie spilling over onto the webpage surrounding it.

But I tried WinLockWindowUpdate() with catastrophic effect (it hung my PM so that I had to reboot), and the invalidation trick didn't work, either. So I'm not sure what else I could try.

Now that you get WM_VRNDISABLED before WM_MOVE, isn't this more a timing problem in mplayer than in the Mozilla code? Are you sure that you can stop the movie so fast that it really doesn't blit any more to the screen before WM_MOVE happens?
(Reporter)

Comment 15

10 years ago
Hi/2.

(In reply to comment #14)
> A difference with our setups might be the smooth scrolling preference (Edit ->
> Preferences -> Content -> Use smooth scrolling in a current SeaMonkey trunk
> build). When that is switched off the problem is more visible and I indeed then
> see a bit of the movie spilling over onto the webpage surrounding it.
> 
> But I tried WinLockWindowUpdate() with catastrophic effect (it hung my PM so
> that I had to reboot), and the invalidation trick didn't work, either. So I'm
> not sure what else I could try.
> 

I think, it didn't hang PM, but prevented PM from being updated.

> Now that you get WM_VRNDISABLED before WM_MOVE, isn't this more a timing
> problem in mplayer than in the Mozilla code? Are you sure that you can stop the
> movie so fast that it really doesn't blit any more to the screen before WM_MOVE
> happens?
> 

Of course, MPlayer can do it. As soon as WM_VRNDISABLED is arrived, blitting is canceled. If this does not work, screen corruption should always occur when moving movie window, but it should not.

I have another suggestion. How about using WinSendMsg() to foreign window instead of WinBroadcastMsg() ? I suspect WinBroadcastMsg() wait until window procedure return.

KO Myung-Hun

(Reporter)

Comment 16

10 years ago
Hi/2.

I've noticed that Flash v7 encounter the scroll problem.

Does Flash v7 use DIVE ? Or it uses Gpi ?

KO Myung-Hun
(Reporter)

Comment 17

10 years ago
(In reply to comment #16)
> Hi/2.
> 
> I've noticed that Flash v7 encounter the scroll problem.
> 

This should be "I've noticed that Flash v7 does not encounter the scroll problem".

Sorry. ^^

> Does Flash v7 use DIVE ? Or it uses Gpi ?
> 
> KO Myung-Hun
> 

(Assignee)

Comment 18

10 years ago
I have no idea what Flash7 really uses. I mean, it certainly uses the Win API internally but how that is translated to PM/GPI or DIVE I have no idea.

Yes, I can try WinSendMsg() which is the function that waits for the message to be processed, see <http://www.warpspeed.com.au/cgi-bin/inf2html.cmd?..\html\book\Toolkt40\PM2.INF+2712>.
(Reporter)

Comment 19

10 years ago
Hi/2.

What is the result of trying WinSendMsg() ?

KO Myung-Hun

(In reply to comment #18)
> I have no idea what Flash7 really uses. I mean, it certainly uses the Win API
> internally but how that is translated to PM/GPI or DIVE I have no idea.
> 
> Yes, I can try WinSendMsg() which is the function that waits for the message to
> be processed, see
> <http://www.warpspeed.com.au/cgi-bin/inf2html.cmd?..\html\book\Toolkt40\PM2.INF+2712>.
> 

(Assignee)

Comment 20

10 years ago
There wasn't any result yet, I was away...
(Assignee)

Comment 21

9 years ago
Sorry for the long delay, I'm getting more and more distracted by other things!

(In reply to comment #19)
> What is the result of trying WinSendMsg() ?

It seems to work. I uploaded a DLL for you to test as <http://temp.weilbacher.org/wdgtos2_sm200804102240_WinSendMsg.zip>. This DLL should work with the latest SeaMonkey nightly from <http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/contrib/2008-04-10-22-trunk/seamonkey-2.0a1pre.en-US.os2.zip>.

At first I was not sure if this is a good idea, in case the plugin hangs the browser would hang, too. But then if a plugin hangs the browser is doomed anyway...
(Assignee)

Comment 22

9 years ago
Created attachment 315402 [details] [diff] [review]
simple fix but using WinSendMsg()

This is the patch that I used for that DLL.
(Reporter)

Comment 23

9 years ago
Hi/2.

(In reply to comment #21)
> Sorry for the long delay, I'm getting more and more distracted by other things!
> 

No problem.

> (In reply to comment #19)
> > What is the result of trying WinSendMsg() ?
> 
> It seems to work. I uploaded a DLL for you to test as
> <http://temp.weilbacher.org/wdgtos2_sm200804102240_WinSendMsg.zip>. This DLL
> should work with the latest SeaMonkey nightly from
> <http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/contrib/2008-04-10-22-trunk/seamonkey-2.0a1pre.en-US.os2.zip>.
> 

Ok. It works fine here, too.

The best is to support VRN message fully, but it's fortunate we could find a workaround.

Thanks.

> At first I was not sure if this is a good idea, in case the plugin hangs the
> browser would hang, too. But then if a plugin hangs the browser is doomed
> anyway...
> 

Good.

KO Myung-Hun
(Assignee)

Comment 24

9 years ago
Comment on attachment 315402 [details] [diff] [review]
simple fix but using WinSendMsg()

Mike, does this workaround look OK to you, too?
Attachment #315402 - Flags: review?(mozilla)
Comment on attachment 315402 [details] [diff] [review]
simple fix but using WinSendMsg()

This looks ok to me.
Attachment #315402 - Flags: review?(mozilla) → review+
(Assignee)

Comment 26

9 years ago
OK, great. Patch checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 27

9 years ago
Patch checked into 1.8 branch.
Keywords: fixed1.8.1.15
(Assignee)

Updated

8 years ago
Attachment #307317 - Attachment is obsolete: true
(Assignee)

Comment 28

8 years ago
Created attachment 391417 [details]
HTML testcase

For the same of reproducing the problem when regressions occur (like now in bug 501618), here is a HTML testcase that shows the bug. The movie file listed in the <embed> statement has to be replaced by one that is actually present on the system (after saving the file to disk).
(Assignee)

Updated

8 years ago
Attachment #391417 - Attachment is patch: false
Attachment #391417 - Attachment mime type: text/plain → text/html
You need to log in before you can comment on or make changes to this bug.