Closed Bug 118877 Opened 23 years ago Closed 23 years ago

[viewpoint] Plugin adjustCursor handler causes mad cursor flicker due to NS_MOUSE_MOVE handling conflict

Categories

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

Product:
Core Graveyard
Component:
Plug-ins
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: jsb, Assigned: peterl-bugs)

References

Details

Attachments

(1 file)

possible fix?
1.67 KB, patch
kmcclusk
: review+
beard
: superreview+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/4.75 [en] (Windows NT 5.0; U)
BuildID:    20011213

We have a plugin that uses handles the adjustCursor event to set the cursor 
(when the cursor is within the bounds of the plugin).  Our handler sets the 
cursor correctly, but it is then immediately reset to the pointer.  The problem 
appears to be as follows:

PresShell::HandleEventInternal() calls manager->PreHandleEvent() prior to 
calling targetContext->HandleDOMEvent(), where HandleDOMEvent passes the event 
to my plugin.  But PreHandleEvent takes a NS_MOUSE_MOVE event and changes the 
cursor unconditionally in that case.  That same NS_MOUSE_MOVE event in turn gets 
passed to my plugin as an adjustCursor event, which sets the cursor back.  Etc.

Reproducible: Always
Steps to Reproduce:
1. Load plugin that changes the cursor in response to an adjustCursor event
2. Wave mouse in plugin

Actual Results:  Cursor flickers between desired image and the pointer

Expected Results:  Cursor should remain at the desired image as set in the 
adjustCursor handler

I've marked the severity high because this is a major usability issue for our 
plugin, and our users are going to have fits if it isn't changed.

Please contact me if you need a copy of our plugin for testing purposes.  
Hopefully it'll be in stable shape by the time you ask...
This sounds similar to a problem Ari at Viewpoint was having, but with
windowless plugins.

Reporter: Can you try an updated, newer build?
Status: UNCONFIRMED → NEW
Ever confirmed: true
No obvious change in 20020109 (source code from ~11 pm last night)
URL: http://n/a
Peter is right- we are having the same problem.  we are a windowless plugin. In 
PreHandleEvent() does the following on mousemove:
    GenerateDragGesture(aPresContext, (nsGUIEvent*)aEvent);
    UpdateCursor(aPresContext, aEvent, mCurrentTarget, aStatus);
    GenerateMouseEnterExit(aPresContext, (nsGUIEvent*)aEvent);
    // Flush reflows and invalidates to eliminate flicker when both a reflow
    // and visual change occur in an event callback. See bug  #36849
    FlushPendingEvents(aPresContext); 

UpdateCursor changes the cursor to an arrow.
FlushPendingEvents sends the event to our plugin, where we change the cursor.
Let me know if you need plugin and testcase.
Should I open a separate bug for PC, or is this enough?
cc:ing some more folks
OS: Mac System 9.x → All
Priority: -- → P1
Hardware: Macintosh → All
Summary: Plugin adjustCursor handler causes mad cursor flicker due to NS_MOUSE_MOVE handling conflict → [viewpoint] Plugin adjustCursor handler causes mad cursor flicker due to NS_MOUSE_MOVE handling conflict
Wow! we have so many cursor bugs I think we need a meta bug to track them all.
:) Hmm, see bug 61955, bug 104970, bug 93145, and... there's one more that both
beard and I commented in which I can't find right now. As I recall, the general
idea of the comments was that when the cursor was over the plugin, we should not
be touching it.
This kinda sounds like 104970. The cursor flickering is important to Viewpoint.

Ari, can you attach a Viewpoint testcase showing the problem? Thanks!
Keywords: edt0.9.4
Priority: P1 → P2
I will attach a test case. Problem is, in our most recent plugins, we disabled 
changing the cursor to avoid the flicker.  I will do a build with it reenabled 
and attach it tomorrow.
Peter, What is the next step here; Is this still needed for 094? My read of
Ari's comment is that Viewpoint has a workaround implemented.
No- we had no workaround.  We just disabled the feature.
As I said, the cursor flickers when the content changes the cursor from arrow 
to something else. We disabled that feature for the CS browser.  We really want 
to reenable it before ship.
I will have a simple test case and plugin with feature enabled this afternoon.
Here is a zipped installer:
C:\Documents and Settings\aberger\Desktop\install\VMPInstaller.zip

It contains an executable installer to run.  If you are using a debug browser 
so the npViewpoint.dll and .xpt don't install into your browser, they are 
included too. Put them in the plugins (not components) folder.
 
Note that this is not our standard installer- it has some debug stuff (like 
dirty rects) enabled.  It may even crash.  It's ok- it's not the shipping 
version.

Here is a test case:
http://cole.viewpoint.com/~compuserve/hidden/cursortest/index.html
Drag the mouse around.  It will probably flicker. It shouldn't.  If you can't 
repeat, please let me know.
sorry: pasted the wrong installer path. you can't get that one...
http://cole.viewpoint.com/~aberger/VMPInstaller200.zip
I would consider the disabled feature in fact a workaround for the failure if
the plugin can in fact ship with the feature disabled. In otherwords is this a
true stopper?
We consider this a blocking bug, as the cursor feedback is critical in our 
plugin to help the user to know what will happen when they click.  The 
workaround of not ever trying to set the cursor is not acceptable.
For the viewpoint plugin, this was not marked as a blocker, just high priority.
-->peterl
Assignee: av → peterl
Keywords: patch
Attached patch possible fix? — — Splinter Review 
With this patch, my cursor doesn't flicker on Ari's testcase on Windows. Still
need to test Mac.

Reporter, do you have a testcase? I'll try those other bugs...
Quick check on Mac looks very good!

Patch looks great! We will reenable the feature. Thanks Peter.
Plussing per verification of fix
Keywords: edt0.9.4 → edt0.9.4+
The patch looks ok but I wonder if, instead of returning an error, you should
instead declare a new cursor type... i.e.

         aTargetFrame->GetCursor(aPresContext, aEvent->point, cursor);
+        if (cursor == NS_STYLE_CURSOR_OVERRIDE)
+          return;  // this frame (probably a plugin) is managing it's own cursor

This seems more obvious to me, but I will gladly defer to someone with more
events experience.
Comment on attachment 66232 [details] [diff] [review]
possible fix?

Patch looks fine to me. I think the comment could be a little clearer. It
should clearly state that it consumes the event when GetCursor fails. 

r=kmcclusk@netscape.com
Attachment #66232 - Flags: review+
*** Bug 112797 has been marked as a duplicate of this bug. ***
Need super review on this and also on bug 120821 (P1, viewpoint painting problem
in absolute DIV/scrolling) and then I can land right away. I sent mail yesterday
to reviewers@mozilla.org and Patrick but I have not heard back yet. Maybe today?
Keywords: edt0.9.4+ → edt0.9.4
I need super review on this and then I can land right away. I sent mail
yesterday to reviewers@mozilla.org and Patrick but I have not heard back yet.
Maybe today?

Comment on attachment 66232 [details] [diff] [review]
possible fix?

sr=beard
Attachment #66232 - Flags: superreview+
Plusing
Keywords: edt0.9.4 → edt0.9.4+
fixed0.9.4
Status: NEW → ASSIGNED
Keywords: fixed0.9.4
in trunk
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
used Ari's testcase/installer. Mouse cursor does not flicker, this works fine. 
verif on 0.9.4 branch build.(01/28)
Keywords: verified0.9.4
.
Status: RESOLVED → VERIFIED
Keywords: fixed0.9.4
See Also: → 1222662
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: