61955 - watch cursor displayed in plugin menus and dialogs

Status: RESOLVED WONTFIX

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

Description

[sean echevarria]

Moz/n6 switches the mouse cursor from the standard arrow to the watch when 
plugins display dialogs or menus.

Test by going to http://www.flash.com and Ctrl+clicking on the flash 
animation.  About 2 or 3 seconds after the menu appears, the cursor changes.

Comment 1

[Henri Sivonen (:hsivonen)]

CCing Pinkerton.

Comment 2

[Mike Pinkerton (not reading bugmail)]

i guess before we send the event into the plugin, we need to turn off the watch
cursor, and turn it back on again when we come out from delivering the event.

look at what i did to turn it off in other areas of the mac widget code.

Comment 3

[Peter Lubczynski]

Petersen, is this a dup or the one you were looking for?

Comment 4

[karnaze (gone)]

Moving to m1.0 and reassigning to sean. sean, if you can't fix this for m1.0, 
please reassign to peterl.

Comment 5

[sean echevarria]

Attachment: patch for embedded plugins

Comment 6

[sean echevarria]

attached patch for the change that Mike suggested - wrapped the ProcessEvent 
call with nsWatchTask::GetTask().Suspend()/nsWatchTask::GetTask().Resume()

The patch relies on a change to the mac project - need to add /gfx/src/mac to 
the path list. nsObjectFrame.cpp now includes nsWatchTask.h from the 
gfx/src/mac directory.

Would appreciate it if someone more mac-savvy could handle that end of the 
checkin.

I couldn't get a full-page plugin to work on my mac - the attached patch may 
not address the problem for full-page plugins.

review please.

Comment 7

[sean echevarria]

Just thought of something.  The patch only addresses mouse invoked actions 
(clicking on a plugin to display a menu or dialog).  It does not address a 
situation like invoking a plugin's 'About' dialog via javascript :(

Comment 8

[Mike Pinkerton (not reading bugmail)]

oh well. i can check this in if you'd like. should i then leave the bug open?

Comment 9

[Peter Lubczynski]

You can try your patch with my full page plugin patch in bug 35682.

Is there something similar to pluginSaftey on the Mac which can trap an OS 
exception. It'd be nice to have this like on Windows.

Comment 10

[Mike Pinkerton (not reading bugmail)]

no, you can't (humanely) trap processor exceptions in c++ on mac.

Comment 11

[Peter Lubczynski]

How does LiveConnect know to invoke if a crash happens?

Comment 12

[sean echevarria]

I don't follow the question.  LiveConnect (in the plugin sense) is no longer 
supported.

Comment 13

[sean echevarria]

re: full-page plugins, the attached patch does not address them.  But it can be 
reviewed and checkedin (along with a modified codewarrior project file) 
independently of the fullpage case.

Comment 14

[Peter Lubczynski]

Sean,

Are you seeking review for your patch?

Comment 15

[Mike Pinkerton (not reading bugmail)]

he sure is, i'm just swamped and haven't r='d it yet (he's in the queue, but near 
the middle)

Comment 16

[Peter Lubczynski]

Mike/Sean, what's the status of this bug?

Comment 17

[sean echevarria]

waiting for review

Comment 18

[Mike Pinkerton (not reading bugmail)]

does this compile? i don't think that nsWatchTask is exported to dist, and 
layout.mcp doesn't look in gfx in the include paths. (my tree is in shambles 
right now, so i can't check).

Comment 19

[sean echevarria]

The patch requires a change to the mac project to get nsWatchTask.h - need to 
add /gfx/src/mac to the path list.  At least that's what I did - I don't know 
if it would be better to change the gfx to export nsWatchTask.h to dist.

I won't be able to checkin that part of the change and will need a mac person 
to take care of it.

Comment 20

[Mike Pinkerton (not reading bugmail)]

no, let's export nsWatchTask.h from gfx. i don't like adding access paths.

Comment 21

[Peter Lubczynski]

Move up milestone as this is annoying and ugly.

Pinkerton, correct me if I'm wrong, but the only thing left to do on this bug is 
export nsWatchTask.h in the MANIFEST file? If that's all, pending a full patch 
is attached, can we get your s/r=?

Comment 22

[Mike Pinkerton (not reading bugmail)]

sure. r=pink with appropriate manifest changes.

Comment 23

[sean echevarria]

Reassigning to Peter.  I won't be able to finish this up.  Needs manifest
changes before it can go in.

Comment 24

[Peter Lubczynski]

Exporting nsWatchTask.h sounds like the wrong way of going as it's a private
class in gfx. cc:ing Don Cone for his input.

I think there is a better way of changing the cursor on the Mac then going so
low level. Isn't there a constant for telling the plugin to change the cursor?
I'm not sure, cc:ing Brian Nesse.

Also moving off to mozilla 0.9.3 as I don't think this is a beta stopper.

Comment 25

[Mike Pinkerton (not reading bugmail)]

it's not that private, we use it all over in widget. it's only in gfx so the
print stuff has access to it.

i think it's ok to export it.

Comment 26

[Brian Nesse (gone)]

I don't know of any built in method for telling a plugin what the cursor should
be or that it's time to change. I would assume that the plugin would be
responsible for the cursor while it's over the plugin and the browser would be
responsible at all other times.

Why are we going to the watch cursor anyway? If you control click on a normal
page we don't go to a watch cursor.

Another good example of this is to go to one of the Qubic VR movies at Apple's
website.

http://www.apple.com/quicktime/products/gallery/church.html

First, note that the movie controls the cursor when the mouse is over the movie
frame. Next, if I control click on the movie, it pops up the Quicktime context
menu. When I click to dismiss that... I get the watch cursor, followed by our
context menu... this is just patently wrong. Clicks in the plugin should only be
handled by the plugin.

I think that we are working on a symptom, not the real problem here.

Comment 27

[Mike Pinkerton (not reading bugmail)]

argh. this is a dead snake. we get the watch because when we go into the OS to
handle a modal dialog or track an OS context menu we're away from the event loop
so our VBL watch task kicks in.

just turn off the watch cursor before going into plugin code and we're fine. the
patch is good.

Comment 28

[Peter Lubczynski]

Exporting nsWatchTask is going to need some moving of CVS files. Plugins already
depend on gfx, can I just fix up the plugin project file to include gfx like
Widget does without touching gfx?

Comment 29

[Mike Pinkerton (not reading bugmail)]

can't you just put in a manifest file and export it to dist?

Comment 30

[Peter Lubczynski]

http://lxr.mozilla.org/seamonkey/source/gfx/src/mac/nsWatchTask.h

...isn't public. There is no MANIFEST. Wouldn't I have to "move" it to the
"public" folder?

Comment 31

[Mike Pinkerton (not reading bugmail)]

or just add a manifest from gfx/src/mac. there's nothing special about 'public'.

but, if that makes you feel nasty (we do it everywhere, not sure why it would), 
you can add an access path into gfx. six of one, right?

smfr? have an opinion here?

Comment 32

[Simon Fraser [no longer active]]

Just export nsWatchTask.h from gfx. It would be nice if it was in a public folder 
(just remove and re-add; there isn't much revision info we care about), but I 
don't care too much. It's not a public interface.

Comment 33

[Peter Lubczynski]

Attachment: patch with MANIFEST and tip changes merged in

Comment 34

[Peter Lubczynski]

Pinkerton & Simon, can I get your final blessing on the last patch attached in
this bug? Thanks!

Comment 35

[Simon Fraser [no longer active]]

You'll also need to edit MozillaBuildList.pm to add your new MANIFEST file so the 
list.

Comment 36

[Peter Lubczynski]

Hm...I updated my tree and fixed up MozillaBuildList.pm, but now this doesn't
seem to work. I get a currupt cursor now. Pushing off to mozilla0.9.4.

Comment 37

[Peter Lubczynski]

*** Bug 95909 has been marked as a duplicate of this bug. ***

Comment 38

[Peter Lubczynski]

nominating nsbeta1

Comment 39

[Jaime Rodriguez, Jr.]

Minusing, setting to 1.2

Comment 40

[Peter Lubczynski]

Classic only --->future

Comment 41

[Simon Paquet [:sipaq]]

This bug is targeted at a Mac classic platform/OS, which is no longer supported
by mozilla.org. Please re-target it to another platform/OS if this bug applies
there as well or resolve this bug.

I will resolve this bug as WONTFIX in four weeks if no action has been taken.
To filter this and similar messages out, please filter for "mac_cla_reorg".

Comment 42

[Simon Paquet [:sipaq]]

Mac classic only. Marking WONTFIX