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




16 years ago
2 years ago


(Reporter: Jonathan Brecher, Assigned: Peter Lubczynski)



Firefox Tracking Flags

(Not tracked)



(1 attachment)

1.67 KB, patch
Kevin McCluskey (gone)
: review+
Patrick C. Beard
: superreview+
Details | Diff | Splinter Review


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

Comment 1

16 years ago
This sounds similar to a problem Ari at Viewpoint was having, but with
windowless plugins.

Reporter: Can you try an updated, newer build?
Ever confirmed: true

Comment 2

16 years ago
No obvious change in 20020109 (source code from ~11 pm last night)

Comment 3

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

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.

Comment 4

16 years ago
Should I open a separate bug for PC, or is this enough?

Comment 5

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

Comment 6

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

Comment 7

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

Comment 8

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

Comment 9

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

Comment 10

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

Comment 11

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

Here is a test case:
Drag the mouse around.  It will probably flicker. It shouldn't.  If you can't 
repeat, please let me know.

Comment 12

16 years ago
sorry: pasted the wrong installer path. you can't get that one...

Comment 13

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

Comment 14

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

Comment 15

16 years ago
For the viewpoint plugin, this was not marked as a blocker, just high priority.

Comment 16

16 years ago
Assignee: av → peterl
Keywords: patch

Comment 17

16 years ago
Created attachment 66232 [details] [diff] [review]
possible fix?

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...

Comment 18

16 years ago
Quick check on Mac looks very good!

Comment 19

16 years ago
Patch looks great! We will reenable the feature. Thanks Peter.

Comment 20

16 years ago
Plussing per verification of fix
Keywords: edt0.9.4 → edt0.9.4+

Comment 21

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

Attachment #66232 - Flags: review+

Comment 23

16 years ago
*** Bug 112797 has been marked as a duplicate of this bug. ***

Comment 24

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

Comment 25

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

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

Attachment #66232 - Flags: superreview+

Comment 27

16 years ago
Keywords: edt0.9.4 → edt0.9.4+

Comment 28

16 years ago
Keywords: fixed0.9.4

Comment 29

16 years ago
in trunk
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 30

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

Comment 31

16 years ago


14 years ago
Keywords: fixed0.9.4


2 years ago
See Also: → bug 1222662
You need to log in before you can comment on or make changes to this bug.