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)

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: jsb, Assigned: peterl-bugs)

References

Details

Attachments

(1 file)

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)
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? (deleted) — 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.4edt0.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.4edt0.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
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: