1234958 - Can't scroll with mouse wheel when mouse over Flash

Status: RESOLVED WONTFIX

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

Description

[Jake]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151221151411

Steps to reproduce:

Move mouse over a flash


Actual results:

You can't scroll the page with mouse wheel any more.


Expected results:

There was a report, but it's clearly not the same on, because my problem only happens with 44.
https://bugzilla.mozilla.org/show_bug.cgi?id=483136

Comment 1

[Jake]

Actually not all of the flashes,some can, this site can't(just take one for example).
http://tv.sohu.com/20151216/n431550482.shtml

Comment 2

[Jake]

If the scrolling starts outside of the flash, it can continue in the flash if you don't move the mouse.

Comment 3

[Jake]

By the way, my flash version is 20.0.0.255, it may have nothing to do with it, because it works fine with 43, just in case.

Comment 4

[Chris Peterson [:cpeterson]]

Thanks for reporting this problem, Jake.

@ Mayauki, does this Flash bug sound like a regression from your WM_MOUSEWHEEL fix for windowless plugins in bug 376679? Is this bug a duplicate of bug 359403?

Comment 5

[Jake]

(In reply to Chris Peterson [PTO-ish] [:cpeterson] from comment #4)
> Thanks for reporting this problem, Jake.
> 
> @ Mayauki, does this Flash bug sound like a regression from your
> WM_MOUSEWHEEL fix for windowless plugins in bug 376679? Is this bug a
> duplicate of bug 359403?

Thanks for replying.
It sure looks like a regression from bug 376679, the timeline fits, my problem started with 44, bug 376679 was fixed at 44.
Could you just open the link I posted with 44 or later version, say if you can scroll?
Thanks again.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

No, it's intentional behavior and only on windowless flash player. It's impossible to confirm if wheel event *will* be consumed by windowless flash player due to OOPP but we need to support wheel events on windowless flash player. So, this behavior is by design even though it's not the best UX.

Comment 7

[Jake]

Ok,I will stick with 43 then, at least now I know it's not a problem on my end.
The forced signing thing is a pain in the neck anyway.
Buy the way, it works fine with Chrome.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I wonder, if we create a pref which kills wheel message support on windowless plugins may help some users. But I'm not sure if adding such hidden pref is worthwhile for most users (I guess that most users never touch about:config).

Comment 9

[Chris Peterson [:cpeterson]]

Masayuki, IIUC dvander and billm are working with Adobe to make Flash only support windowless mode. This plugin change should reduce browser jank and deadlocks. Would that mean all Flash content will "eat" the mouse wheel events and break scrolling?

Comment 10

[Karl Tomlinson (:karlt)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline until 1/6) from comment #6)
> It's
> impossible to confirm if wheel event *will* be consumed by windowless flash
> player due to OOPP but we need to support wheel events on windowless flash
> player. So, this behavior is by design even though it's not the best UX.

The ipc message has a response, so I don't know why OOPP would mean that the event *will* be consumed.

ISTR that Flash Player often says events are consumed even when they are not.
The reason was something like registering an event handler in the flash script will mean that the player always says it is consumed even if the event handler is not consuming the particular event.  Even so, respecting the response may permit scrolling over plugins that don't have any scroll event handlers.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Chris Peterson [:cpeterson] from comment #9)
> Would that mean all Flash content will "eat" the mouse wheel
> events and break scrolling?

Unfortunately, yes.

(In reply to Karl Tomlinson (ni?:karlt) from comment #10)
> (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline until 1/6)
> from comment #6)
> > It's
> > impossible to confirm if wheel event *will* be consumed by windowless flash
> > player due to OOPP but we need to support wheel events on windowless flash
> > player. So, this behavior is by design even though it's not the best UX.
> 
> The ipc message has a response, so I don't know why OOPP would mean that the
> event *will* be consumed.

Oh, really? If we receive the result (i.e., whether a mouse wheel event is consumed or not) synchronously, we can fallback to normal path.

> ISTR that Flash Player often says events are consumed even when they are not.
> The reason was something like registering an event handler in the flash
> script will mean that the player always says it is consumed even if the
> event handler is not consuming the particular event.  Even so, respecting
> the response may permit scrolling over plugins that don't have any scroll
> event handlers.

Yeah, an Adobe engineer said that if swf content listens "mousewheel" events, it means that the content consumes the event (i.e., not related if the handler calls .peventDefault()). He claimed that it's by design, not a bug.

Comment 12

[Karl Tomlinson (:karlt)]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (offline until 1/6) from comment #11)
> (In reply to Karl Tomlinson (ni?:karlt) from comment #10)
> > The ipc message has a response, so I don't know why OOPP would mean that the
> > event *will* be consumed.
> 
> Oh, really? If we receive the result (i.e., whether a mouse wheel event is
> consumed or not) synchronously, we can fallback to normal path.

Yes, it should be possible to do whatever we would do in process.  See

https://dxr.mozilla.org/mozilla-central/rev/1ec3a3ff68f2d1a54e6ed33e926c28fee286bdf1/dom/plugins/ipc/PPluginInstance.ipdl#102
https://dxr.mozilla.org/mozilla-central/rev/1ec3a3ff68f2d1a54e6ed33e926c28fee286bdf1/dom/plugins/ipc/PluginInstanceChild.cpp#872

It depends on plugin to return a sensible value, but even if it only does that sometimes, it would be worth doing what we can.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thanks. I'll try to fix this.

Comment 15

[Masatoshi Kimura [:emk]]

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #8)
> I wonder, if we create a pref which kills wheel message support on
> windowless plugins may help some users. But I'm not sure if adding such
> hidden pref is worthwhile for most users (I guess that most users never
> touch about:config).

Could you add a pref if the proper fix will take a long time? I'm very frustrated whenever I have to enable Flash. Also, niconico no longer uses Flash for the scrollable comment view.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Also, niconico no longer uses Flash for the scrollable comment view.

Oh, I confirmed it. Hmm, I need to look for a textcase for mouse wheel on Flash, sigh...

Basically, we shouldn't kill wheel support in default settings, but it's possible to add a pref.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Got it: http://hakuhin.jp/as/mouse.html#MOUSE_06

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Separate wheel event's default action handler to independent methods

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.2 Default action of wheel event should be handled by ourselves if plugin doesn't handle it

Hmm, looks like PluginInstanceOwner::ProcessEvent() returns nsEventStatus_eIgnore even if the Flash Player handles wheel event. I'll confirm it and need to contact adobe if Flash Player needs to change its behavior...

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e11577f3d726

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Okay, I confirmed that PluginInstanceParent::NPP_HandleEvent() always returns false (at least for wheel events) even with calling CallNPP_HandleEvent().
http://mxr.mozilla.org/mozilla-central/source/dom/plugins/ipc/PluginInstanceParent.cpp?rev=75dfe10ec44a#1730


karlt: Do you know an expert around this or this is explicitly Adobe needs to work for this?

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Separate wheel event's default action handler to independent methods

updated for bug 1250050.

Comment 23

[Karl Tomlinson (:karlt)]

I don't have a contact at Adobe, sorry.  (It was many years ago that I last looked at plugin event handling.)  Perhaps Josh can suggest a contact?

We won't be able to determine whether Flash handles the event if it doesn't tell us.

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Karl Tomlinson (ni?:karlt) from comment #23)
> I don't have a contact at Adobe, sorry.  (It was many years ago that I last
> looked at plugin event handling.)  Perhaps Josh can suggest a contact?

Thank you. Now, we have a pipe with Adobe, because we're are working on all Flash Player forcibly windowlessed in Windows x64 build. So, we can contact Adobe easy.

I was thinking that whether this is really Flash Player's fault. I'm not 100% sure it. Therefore, I'd like somebody check if it's not our fault (e.g., there are some hack for each event type).

Comment 25

[Karl Tomlinson (:karlt)]

PluginInstanceChild::WinlessHandleEvent() is probably the best place to check what the plugin is returning.

https://hg.mozilla.org/mozilla-central/annotate/d848a5628d801a460a7244cbcdea22d328d8b310/dom/plugins/ipc/PluginInstanceChild.cpp#l2186

Documentation is at
https://developer.mozilla.org/en-US/Add-ons/Plugins/Reference/NPP_HandleEvent
https://developer.mozilla.org/en-US/Add-ons/Plugins/Reference/NPEvent

I don't know of any special cases for the event handling.

Comment 26

[FateRover]

still reproducible with ff 45,any progress？

Comment 27

[Masatoshi Kimura [:emk]]

(In reply to FateRover from comment #26)
> still reproducible with ff 45,any progress？

Bug 1250050 added an workaround for this bug. It is usable only if you don't use any Flash apps that depend on mouse wheel. You can flip "plugin.mousewheel.enabled" pref to enable the workaround.
Unfortunately this workaround will not be available until Firefox 46. Too late for Firefox 45, sorry.

The proper fix will need a work on Adobe side. That said, is it possible to land our change first? Otherwise Adobe can't test the Flash update.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> The proper fix will need a work on Adobe side. That said, is it possible to land our change first?
> Otherwise Adobe can't test the Flash update.

First, I need to check comment 25. However, I don't have much time now. After current work (bug 1128771), I'll be back here.

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that we don't need to work more for interaction with Flush unless it's really critical to use the Flush content. Feel free to reopen this if you have some patches.