CancelMouseOver not working as expected

Posted by Fiendish on Fri 27 May 2011 06:47 PM — 33 posts, 123,527 views.

Fiendish USA Global Moderator Fri 27 May 2011 06:47 PM #0

WindowInfo option 19 claims to show

Quote:

Hotspot ID of the hotspot currently being moused-over (empty string if none)

Except if you call it from a CancelMouseOver routine, the reported hotspot is the one you just left rather than the one (or none) you might be in now.

It looks like my CancelMouseOver is being called before the mouse actually leaves the hotspot area. This is unintuitive behavior to me.

Twisol USA Fri 27 May 2011 08:13 PM #1

I agree. In general, callbacks should be executed after relevant state has been modified, so the code has the most up-to-date view of what's going on.

Nick Gammon Australia Forum Administrator Sat 28 May 2011 12:20 AM #2

I think it is done like that because this is the code that calls CancelMouseOver:


      // cancel previous move-over hotspot
      if (!old_mw->m_sMouseOverHotspot.empty ())   // HotspotId was used
        {
        // lookup that HotspotId
        HotspotMapIterator it = old_mw->m_Hotspots.find (old_mw->m_sMouseOverHotspot);

        // call CancelMouseOver for that hotspot, if it exists
        if (it != old_mw->m_Hotspots.end ())
          {
          RemoveToolTip ();
          old_mw->m_bExecutingScript = true;
          Send_Mouse_Event_To_Plugin (it->second->m_dispid_CancelMouseOver,
                                      old_mw->m_sCallbackPlugin,
                                      it->second->m_sCancelMouseOver, 
                                      old_mw->m_sMouseOverHotspot);
          old_mw->m_bExecutingScript = false;
          }
        old_mw->m_sMouseOverHotspot.erase ();  // no mouse-over right now
        m_sPreviousMiniWindow.erase ();  // no longer have a previous mouse-over
        }   // we had previous hotspot

The line in bold is where it is erased, and if that was moved up earlier then it wouldn't know which hotspot to notify is no longer active. Are you two suggesting something like this?


      // cancel previous move-over hotspot
      if (!old_mw->m_sMouseOverHotspot.empty ())   // HotspotId was used
        {
        // lookup that HotspotId
        HotspotMapIterator it = old_mw->m_Hotspots.find (old_mw->m_sMouseOverHotspot);
        
        string  sOldMouseOverHotspot = old_mw->m_sMouseOverHotspot;
        old_mw->m_sMouseOverHotspot.erase ();  // no mouse-over right now


        // call CancelMouseOver for that hotspot, if it exists
        if (it != old_mw->m_Hotspots.end ())
          {
          RemoveToolTip ();
          old_mw->m_bExecutingScript = true;
          Send_Mouse_Event_To_Plugin (it->second->m_dispid_CancelMouseOver,
                                      old_mw->m_sCallbackPlugin,
                                      it->second->m_sCancelMouseOver, 
                                      sOldMouseOverHotspot);
          old_mw->m_bExecutingScript = false;
          }
        m_sPreviousMiniWindow.erase ();  // no longer have a previous mouse-over
        }   // we had previous hotspot

It's more complicated than that because later on it also uses old_mw->m_sMouseOverHotspot but I supposed conceptually it could work.

Twisol USA Sat 28 May 2011 01:04 AM #3

Close, but instead of clearing the currently moused-over hotspot, instead set it to the hotspot (if any) that the mouse is currently over. Conceptually, that means moving the whole CancelMouseOver section to after the hotspot is updated.

Nick Gammon Australia Forum Administrator Sat 28 May 2011 01:34 AM #4

Bearing in mind what I said about needing it for the callbacks. So perhaps this:

Remember current hotspot name in old miniwindow, if any old miniwindow.
Remember current hotspot name in new miniwindow, if any new miniwindow.
Set "current hotspot" in both old miniwindow, and new miniwindow (if either exists) to the appropriate value (which might be empty).
Do the various callbacks for the old hotspot (eg. CancelMouseOver) using the saved hotspot name.

Twisol USA Sat 28 May 2011 02:02 AM #5

Exactly.

Fiendish USA Global Moderator Sat 28 May 2011 03:37 AM #6

Nick Gammon said:

Bearing in mind what I said about needing it for the callbacks. So perhaps this:

*Remember current hotspot name in old miniwindow, if any old miniwindow.

*Remember current hotspot name in new miniwindow, if any new miniwindow.

I don't quite understand this progression, since I don't know what "current" is referring to here. But I'm going to assume that at least the first "current" here means the hotspot that is being exited rather than the one being (potentially) entered, and that the second current means the hotspot being (potentially) entered.

Quote:
Do the various callbacks for the old hotspot (eg. CancelMouseOver) using the saved hotspot name.

Don't forget the callbacks for the (potential) new hotspot (MouseOver).

Nick Gammon Australia Forum Administrator Sat 28 May 2011 04:29 AM #7

"Current" means the one that initiated the sequence. For example:

You mouse-over a hotspot. It remembers the hotspot ID that is now the "current moused over" hotspot. This raises a "mouse over" callback.
You move the mouse onto a different hotspot. First it needs to raise a "cancel mouseover" for the current (ie. the original) hotspot ... if any. Then it raises a "mouse over" for the new hotspot (and makes it the current one). The original one had to be remembered so we could pair the mouse-over with the cancel-mouse-over for the original hotspot.

Now your complaint, I gather, is that during the "cancel the original mouse-over" handling, it hasn't switched Info option 19 to the new hotspot. You can see why it might work that way, I hope. Effectively the code is "finishing off" the old hotspot. However I agree you might not expect it to behave exactly that way.

Quote:

Don't forget the callbacks for the (potential) new hotspot (MouseOver).

Well you get that now, don't you? And I think it is correct for the new one:


      // if different hotspot from before
      if (sHotspotId != mw->m_sMouseOverHotspot)
        {
        // this is our new one
        mw->m_sMouseOverHotspot = sHotspotId;
        mw->m_bExecutingScript = true;
        Send_Mouse_Event_To_Plugin (pHotspot->m_dispid_MouseOver,
                                    mw->m_sCallbackPlugin, 
                                    pHotspot->m_sMouseOver, 
                                    sHotspotId);
        mw->m_bExecutingScript = false;

Fiendish USA Global Moderator Sat 28 May 2011 04:54 AM #8

Nick Gammon said:
Well you get that now, don't you?

Of course. I just thought the list looked wrong without stating it.

Quote:
Now your complaint, I gather, is that during the "cancel the original mouse-over" handling, it hasn't switched Info option 19 to the new hotspot.

Right. I think this goes likewise for some of the other windowinfo options, like X/Y mouse position. I'd want to know the position outside the hotspot. Right now it looks like cancelmouseover has been processed before the mouse has left the area instead of after the mouse has left, which is what I hope for.

Nick Gammon Australia Forum Administrator Sat 28 May 2011 05:41 AM #9

Well I think I found a bug that you didn't. Muahahaha.

Can you confirm this?

According to this page:

http://www.gammon.com.au/mushclient/mw_hotspots.htm

You are supposed to get pairs of events, like this:


mouse-over -> cancel-mouse-over

and


mouse-down -> cancel-mouse-down OR mouse-up

But I just noticed this:


mouse-over ... cancel-mouse-over ... mouse-down ... mouse-up

So far so good. But then move the mouse out of the hotspot and get:


cancel-mouse-over

So that's an orphan cancel-mouse-over. If you mouse-down, and move the mouse out of the hotspot, you just get the cancel-mouse-down.

In fact I'm inclined to think that if you move the mouse over a hotspot, then click and release it, you should then get:


mouse-over ... cancel-mouse-over ... mouse-down ... mouse-up ... mouse-over

Because the mouse is over the hotspot, right? Then the cancel-mouse-over is no longer an orphan.

What do you think?

Fiendish USA Global Moderator Sat 28 May 2011 05:58 AM #10

Quote:
you should then get:
mouse-over ... cancel-mouse-over ... mouse-down ... mouse-up ... mouse-over

This is what I see already. Not sure why it isn't working for you. :\

If anything, I think it's a little weird for cancel-mouse-over to get called just because you've clicked. If I had my way, I think I'd want the progression for *move the mouse over a hotspot, click, release, move the mouse away* to be

mouse-over ... mouse-down ... mouse-up ... cancel-mouse-over

and for *move the mouse over a hotspot, click, move the mouse away, release* to be

mouse-over ... mouse-down ... cancel-mouse-over ... cancel-mouse-down

but that probably adds a fair deal of complexity

Nick Gammon Australia Forum Administrator Sat 28 May 2011 06:32 AM #11

I'm getting inconsistent results now. However the code doesn't do it automatically. The mouse-up code does not generate a mouse-over. However a slight movement afterwards (or maybe a queued movement) seems to do it.

I can't really account for it. The release version seems to generate the extra call most of the time, but it simply isn't in the code to do it, on the mouse-up event.

Fiendish USA Global Moderator Sat 28 May 2011 06:52 AM #12

I doubt that it's a slight movement on my part, since I'm using a laptop and the buttons are not physically linked to the touch sensor.

Nick Gammon Australia Forum Administrator Sat 28 May 2011 08:01 AM #13

It seems to sometimes do it and sometimes not for me, even if I hold the mouse still with one hand.

Anyway, I think I have fixed it (commit 5334bac), but am not totally confident. Twisol could apply the patch and see if it works better or worse than before. If you like, Fiendish, I can upload the revised executable in a pre-release form for you to test.

Fiendish USA Global Moderator Sat 28 May 2011 08:12 AM #14

I'll be happy to test it out, though I'm a bit scared that I'm going to start getting multiple mouseovers now after mouseup.

Nick Gammon Australia Forum Administrator Sat 28 May 2011 08:53 AM #15

Well you said you got one anyway, and the way it is written you will only ever get one.

Nick Gammon Australia Forum Administrator Sat 28 May 2011 11:55 PM #16

Fiendish I seem to have misplaced your email address. If you email me I'll give you a link to the beta test version with the CancelMouseOver fixes in it.

[EDIT] I sent a forum message to you, if your email address on the forum is correct you should get that.

Fiendish USA Global Moderator Sun 29 May 2011 01:54 AM #17

I got it, thanks.

[EDIT] WindowInfo option 19 appears to work as expected now. And cancelmouseovers still work properly, so that's good.

Nick Gammon Australia Forum Administrator Sun 29 May 2011 03:29 AM #18

OK good. Well if you could keep testing this version please to confirm that the new things work the way you want them to, and existing stuff isn't broken. Thanks.

Twisol USA Sun 29 May 2011 09:33 PM #19

I found a pretty decent bug. Click down on a hotspot, then move outside the window and release. Then mouse over the hotspot (or any hotspot) and the mouseover event won't fire.

Inside (0,9) - current: (0,9)
Outside (0,9) - current: 
Clicking (0,9) - current: 
Cancelling (0,9) - current: (0,9)

"Cancelling" is the cancelmousedown event (cancelmouseover is "Outside"), and current shows the return value of WindowInfo option 19. I'm guessing that when you release the mouse button, MUSHclient sets the current hotspot back to what it was before the click, since it clears option 19 during a click. Since MUSHclient thinks you're over a hotspot, it never emits another mouseover event. This situation clears itself if you click and release within the same hotspot afterwards.

It appears also that you need to move the mouse completely outside of the miniwindow for this to happen. Moving it elsewhere in the window doesn't do it.

For the record, I'm not entirely sure I understand why MUSHclient emits a cancelmouseover when you click, either.

Twisol USA Sun 29 May 2011 09:56 PM #20

Here's the plugin I used to test the changes.

http://dl.dropbox.com/u/10356966/HotspotTest.plugin.zip

Nick Gammon Australia Forum Administrator Sun 29 May 2011 11:51 PM #21

OK fixed that. The corrected version is in the same place except change "beta" to "beta2".

My test code btw is:


win = "test_" .. GetPluginID ()  -- get a unique name, ensure not empty if outside plugin
WindowCreate (win, 0, 0, 400, 400, miniwin.pos_center_all, 0, ColourNameToRGB("white"))  -- create window
WindowShow (win,  true)  -- show it 


-- Grid
for i = 1, math.max (WindowInfo (win, 3), WindowInfo (win, 4)) / 20 do
  WindowLine (win, i * 20, 0, i * 20, WindowInfo (win, 4), 0xC0C0C0, miniwin.pen_solid, 1)
  WindowLine (win, 0, i * 20, WindowInfo (win, 3), i * 20, 0xC0C0C0, miniwin.pen_solid, 1)
end -- for


function showhotspot ()
  ColourNote ("gray", "", "  Current hotspot over = " .. WindowInfo (win, 19))
  ColourNote ("gray", "", "  Current hotspot down = " .. WindowInfo (win, 20))
end -- showhotspot

function mouseover (flags, hotspot_id)
  Note ("we moused over hotspot " .. hotspot_id)
  showhotspot ();
  
end -- mouseover

function cancelmouseover (flags, hotspot_id)
  Note ("we cancelled mouseover hotspot " .. hotspot_id)
  showhotspot ();
end -- cancelmouseover 

function mousedown (flags, hotspot_id)
  Note ("we moused down over hotspot " .. hotspot_id)
  showhotspot ();
end -- mousedown

function cancelmousedown (flags, hotspot_id)
  Note ("we cancelled mouse down over hotspot " .. hotspot_id)
  showhotspot ();
end -- cancelmousedown 

function mouseup (flags, hotspot_id)
  Note ("we moused up over hotspot " .. hotspot_id)
  showhotspot ();
end -- mouseup 

WindowAddHotspot(win, "hs1",  
                 10, 10, 60, 80,   -- rectangle
                 "mouseover", 
                 "cancelmouseover", 
                 "mousedown",
                 "cancelmousedown", 
                 "mouseup", 
                 "Hot Spot 1",  -- tooltip text
                 miniwin.cursor_hand, 0)  -- hand cursor


WindowRectOp ( win, miniwin.rect_frame, 10, 10, 60, 80, ColourNameToRGB ("red"), 0)


WindowAddHotspot(win, "hs2",  
                 50, 10, 160, 100,   -- rectangle
                 "mouseover", 
                 "cancelmouseover", 
                 "mousedown",
                 "cancelmousedown", 
                 "mouseup", 
                 "Hot Spot 2",  -- tooltip text
                 miniwin.cursor_hand, 0)  -- hand cursor


WindowRectOp ( win, miniwin.rect_frame, 50, 10, 160, 100, ColourNameToRGB ("blue"), 0)

Nick Gammon Australia Forum Administrator Sun 29 May 2011 11:54 PM #22

The fix was pretty minor:

https://github.com/nickgammon/mushclient/commit/c87c92ea8a98e0

I keep getting confused about the difference between emptying a string and erasing it.


-      it->second->m_sMouseDownHotspot.empty ();
+      it->second->m_sMouseDownHotspot.erase ();

[EDIT]

Well spotted though!

Twisol USA Mon 30 May 2011 12:54 AM #23

That's working again, but WindowInfo 19 should be empty during that cancelmousedown event, since you're letting go of the mouse outside the window.

Nick Gammon said:
I keep getting confused about the difference between emptying a string and erasing it.

Heheh, yes, ::empty() just checks if it's empty or not. I'm glad Ruby lets you use names like empty? and destroy!, it really helps...

Nick Gammon said:
Well spotted though!

Thanks!

Nick Gammon Australia Forum Administrator Mon 30 May 2011 01:00 AM #24

Twisol said:

That's working again, but WindowInfo 19 should be empty during that cancelmousedown event, since you're letting go of the mouse outside the window.

It is, isn't it?

Twisol USA Mon 30 May 2011 02:06 AM #25

Cancelling (0,9) - current: (0,9)
Nope.

Twisol USA Mon 30 May 2011 02:39 AM #26

Strange. Sometimes it is, sometime it isn't. I haven't found any pattern to it yet.

Nick Gammon Australia Forum Administrator Mon 30 May 2011 04:41 AM #27

Another good catch!

I hope I fixed that.

The corrected version is in the same place except change "beta2" to "beta3".

Twisol USA Mon 30 May 2011 04:57 AM #28

Hmm, I'm still seeing it. Most of the time it shows the hotspot name, and only occasionally is it clear.

Nick Gammon Australia Forum Administrator Mon 30 May 2011 06:50 AM #29

<sigh>

Oh well, try file "beta4".

It seems I was a little aggressive in storing the mouse-over hotspot when you moved the mouse - it was filling that field in, even if the mouse was down.

Twisol USA Mon 30 May 2011 08:07 AM #30

I get a "Page not found" page if I try to get beta4. (For that matter, if I try to get any of the earlier ones too.)

Nick Gammon Australia Forum Administrator Mon 30 May 2011 08:15 AM #31

Well that's odd. It vanished. I put it back.

Twisol USA Mon 30 May 2011 10:24 AM #32

Looking good! I can't reproduce the bug anymore, and everything seems to behave as expected.