Closed Bug 1128238 Opened 5 years ago Closed 5 years ago

[e10s] Facebook's Flash video player does not respond to mouse input

Categories

(Core :: Plug-ins, defect)

x86_64
Windows 10
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla40
Tracking Status
e10s m6+ ---
firefox40 --- verified

People

(Reporter: josh.tumath+bugzilla, Assigned: jimm)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150131030230

Steps to reproduce:

Find a video on Facebook and hover over it to try to reveal the video controls.


Actual results:

The controls may appear for a split second, but disappear again. They may respond better with a mouse click, but again that will only work for a split second.
tracking-e10s: --- → ?
OS: Windows NT → Windows 10
Hardware: x86 → x86_64
Flags: needinfo?(jmathies)
Looks like there are still some issues with mouse coords and windowless.
Flags: needinfo?(jmathies)
major plugin bugs are aurora uplift blockers.
Assignee: nobody → jmathies
Blocks: e10s-plugins
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 1094304
Summary: [e10s] Facebook's Flash video player does not respond to mouse hover → [e10s] Facebook's Flash video player does not respond to mouse input
Duplicate of this bug: 1139574
It appears that we're not adjusting the coordinates for mouse events for the chrome correctly here - 

https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsPluginFrame.cpp#764

In non-e10s, windowless delivers events relative to the browser window. In e10s, our coords are relative to content.

However, adjusting this doesn't fix the entire problem. Flash does some funny stuff behind the scene with mouse input. I see a series of api calls when mouse input is processed:

GetCursorPos
WindowFromPoint
ScreenToClient
..

Still doing some compares between e10s and non-e10s here to try and figure out where flash is going wrong.
Attached patch wip (obsolete) — Splinter Review
Not complete yet, flash still has an issue with origin and the lower youtube controls. Other plugins seems fine.
Attached patch wip (obsolete) — Splinter Review
Flash is doing something under the hood here which messes mouse move events up, not sure at this point what it is they do. I see a few common calls by the flash library in processing mouse input:

1) flash calls GetCursorPos, which returns cursor position in screen coordinates
2) flash call WindowFromPoint using the point returned above
3) flash checks the window handle returned by WindowFromPoint to see if it's visible
4) flash calls ScreenToClient([hwnd from WindowFromPoint], [point from GetCursorPos])

In step four, they get back a point equal to the point we hand in through handleevent.
Attachment #8578705 - Attachment is obsolete: true
benjamin, do we have any contacts at adobe who might be able to help poke at this from their end?
Flags: needinfo?(benjamin)
Flags: needinfo?(benjamin)
Attachment #8578762 - Attachment is patch: true
Duplicate of this bug: 1145967
Duplicate of this bug: 1146071
This is ADBE 3959281
Keywords: leave-open
Lets get this part landed since I'm confident we need it.
Attachment #8578762 - Attachment is obsolete: true
Attachment #8586322 - Flags: review?(aklotz)
Attachment #8586322 - Flags: review?(aklotz) → review+
thanks for the quick review!

try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aec320da560c
This bug occurs on most video players on my end, even youtube.
Depends on: 1151310
Blocks: 1151310
No longer depends on: 1151310
Attachment #8586322 - Attachment is obsolete: true
Attached patch patch v.2Splinter Review
Same basic patch except I'm adjusting coords in slightly different places this time. This completely fixes the problem in flash for me, without breaking silverlight afaict.
Duplicate of this bug: 1148017
No longer blocks: 1151310
Duplicate of this bug: 1151310
Attachment #8589633 - Flags: review?(aklotz)
FYI, I changed the name of nsPluginFrame::GetTabChromeOffset() to GetRemoteTabChromeOffset() to prevent confusion. I don't want people calling that in non-e10s and expecting it to return a good value.
Attachment #8589633 - Flags: review?(aklotz) → review+
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/9902d3bf59e4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Depends on: 1153872
Flags: qe-verify+
QA Contact: cornel.ionce
Confirming this fix on latest DevEdition, build ID:20150720004006 and latest Nightly, build ID: 20150720030213.
The video controls are now properly displayed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.