1003943 - [e10s] Window resize mouse icon persists when entering content

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Steven MacLeod [:smacleod]]

mconley was seeing this on Windows 7, but niether of us could reproduce on OSX

To reproduce:

a) Open e10s window
b) Hover mouse over right side of window, displaying window resize mouse icon and leave it there for a couple seconds.
c) Move mouse into content
d) Mouse will continue to use resize icon for several seconds.

Comment 1

[Steven MacLeod [:smacleod]]

The underlying problem here might also relate to Bug 1003934

Comment 2

[Jim Mathies [:jimm]]

Attachment: wip patch

Comment 3

[Jim Mathies [:jimm]]

Attachment: wip patch

Comment 4

[Jim Mathies [:jimm]]

Attachment: clear cached cursor constants in widget when crossing remote frames v.1

With remote content we have a problem with cached cursors in widget interfering with cursor updates:

1) cursor moves around in a remote frame, PuppetWidget caches its current cursor in mCursor (eCursor_standard).
2) cursor moves out of the remote frame into chrome, platform widget caches its current cursor in mCursor. This could be any cursor type.
3) cursor moves back to remote content, esm calls SetCursor(eCursor_standard) - cached version in PuppetWidget matches so the cursor isn't actually updated.

result: we get stuck with the last chrome cursor in remote content.

This is easy to reproduce on Win7 by dragging back and forth across the transparent chrome window border.

Comment 5

[Neil Deakin]

Comment on attachment 8426226 [details] [diff] [review]
clear cached cursor constants in widget when crossing remote frames v.1

>diff -r 64f1c03dd6ec dom/events/EventStateManager.cpp
>--- a/dom/events/EventStateManager.cpp	Wed May 21 08:24:25 2014 -0500
>+++ b/dom/events/EventStateManager.cpp	Wed May 21 08:29:55 2014 -0500
>@@ -569,16 +569,28 @@
>       mouseEvent->reason = WidgetMouseEvent::eSynthesized;
>       // then fall through...
>     } else {
>       GenerateMouseEnterExit(mouseEvent);
>       //This is a window level mouse exit event and should stop here
>       aEvent->message = 0;
>       break;
>     }
>+  case NS_MOUSE_EXIT_SYNTH:
>+    // If this is a remote frame, we receive NS_MOUSE_EXIT_SYNTH when the

Isn't this called on every mouseexit from every node? You might ask Olli about this.


>   virtual nsCursor        GetCursor();
>   NS_IMETHOD              SetCursor(nsCursor aCursor);
>   NS_IMETHOD              SetCursor(imgIContainer* aCursor,
>                                     uint32_t aHotspotX, uint32_t aHotspotY);
>+  virtual void            ClearCachedCursor() { mCursor = eCursor_nocache; }

What happens when a caller tries to retrieve the cursor? Won't it return eCursor_nocache?

Comment 6

[Jim Mathies [:jimm]]

(In reply to Neil Deakin from comment #5)
> Comment on attachment 8426226 [details] [diff] [review]
> clear cached cursor constants in widget when crossing remote frames v.1
> 
> >diff -r 64f1c03dd6ec dom/events/EventStateManager.cpp
> >--- a/dom/events/EventStateManager.cpp	Wed May 21 08:24:25 2014 -0500
> >+++ b/dom/events/EventStateManager.cpp	Wed May 21 08:29:55 2014 -0500
> >@@ -569,16 +569,28 @@
> >       mouseEvent->reason = WidgetMouseEvent::eSynthesized;
> >       // then fall through...
> >     } else {
> >       GenerateMouseEnterExit(mouseEvent);
> >       //This is a window level mouse exit event and should stop here
> >       aEvent->message = 0;
> >       break;
> >     }
> >+  case NS_MOUSE_EXIT_SYNTH:
> >+    // If this is a remote frame, we receive NS_MOUSE_EXIT_SYNTH when the
> 
> Isn't this called on every mouseexit from every node? You might ask Olli
> about this.

We just finished discussing this in the parent bug, I'm switching to NS_MOUSE_EXIT since the synth event is specific to a dom event, vs. a general purpose widget event. This seems more appropriate since it should only happen when crossing the widget boundary. 
 
> >   virtual nsCursor        GetCursor();
> >   NS_IMETHOD              SetCursor(nsCursor aCursor);
> >   NS_IMETHOD              SetCursor(imgIContainer* aCursor,
> >                                     uint32_t aHotspotX, uint32_t aHotspotY);
> >+  virtual void            ClearCachedCursor() { mCursor = eCursor_nocache; }
> 
> What happens when a caller tries to retrieve the cursor? Won't it return
> eCursor_nocache?

Hmm, good catch. Looks like we have one call to that in dom window utils, afaict, and no calls to that api in the mc or addons code base. I'm tempted to just depreciate it, but maybe I can find a reasonable work around. The right way would be to query the os I guess for the current cursor and compare, but that seems way too involved for an api nobody is currently using.

Comment 7

[Jim Mathies [:jimm]]

a "force update" bool flag in base widget should work.

Comment 8

[Jim Mathies [:jimm]]

Attachment: wip

Comment 9

[Jim Mathies [:jimm]]

Attachment: patch v.1

follow up post changes in the parent bug. smaug, neil suggested you should take a look at this too.

Comment 10

[Jim Mathies [:jimm]]

Comment on attachment 8427168 [details] [diff] [review]
patch v.1

argh, wrong patch file.

Comment 11

[Jim Mathies [:jimm]]

Attachment: patch v.1

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8427170 [details] [diff] [review]
patch v.1

So only PuppetWidget ever sets mUpdateCursor to false, yet we use that
member variable also in other widget implementations. That is odd.

Comment 13

[Jim Mathies [:jimm]]

(In reply to Olli Pettay [:smaug] from comment #12)
> Comment on attachment 8427170 [details] [diff] [review]
> patch v.1
> 
> So only PuppetWidget ever sets mUpdateCursor to false, yet we use that
> member variable also in other widget implementations. That is odd.

ah good catch. I missed the qt and gtk resets. The rest don't cache in mCursor.

Comment 14

[Jim Mathies [:jimm]]

Attachment: patch v.2

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8427247 [details] [diff] [review]
patch v.2

Tiny bit ugly, but I guess this should work.
(we really should have this kind of state handling only in one place, like
in BaseWidget, but I don't have a good suggestion how to have such setup here.)

Comment 16

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/fx-team/rev/fda0bc06b42b

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/fda0bc06b42b

Comment 18

[kenkon]

[bugday-20140716]

This bug is definitely present in the latest Aurora32.0a2 and Nightly33.0a1 builds.

You just need to move the mouse pointer *quickly* into content.

Try it in build 20140715004003 for example. I could not find any build that works actually.

(note: unlike the reporter, I tested Firefox 32-bit but on Win7 *64-bit*)

Comment 19

[Andrei Vaida [:avaida]]

I was able to reproduce the initial issue on Nightly 32.0a1 (2014-04-30) using Windows 7 64-bit.

This is now verified fixed on Firefox 32 Beta 8 (Build ID: 20140818191513), Aurora 33.0a2 (2014-08-20) and Nightly 34.0a1 (2014-08-20).