Closed Bug 520462 Opened 15 years ago Closed 15 years ago

No wheel scrolling anymore after playing embedded movie.

Categories

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

Product:
Core Graveyard
Component:
Plug-ins
Platform:
x86
Windows Vista
Type:
defect

Tracking

(status1.9.2 beta1-fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.3a1
Tracking Flags:
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: ria.klaassen, Assigned: roc)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(2 files)

fix
(deleted), patch
karlt
: review+
Details | Diff | Splinter Review
test
(deleted), patch
jimm
: review+
Details | Diff | Splinter Review 
STR:

- Go to http://zaplog.nl/zaplog/article/draadloos_netwerksignalen_laten_je_door_muren_kijken#rss 
- Scroll down and start flash video
- Click Stop and click outside movie
- Attempt to scroll down with mouse wheel

Actual result: page doesn't scroll anymore.

Regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8cbc47eee659&tochange=67e02399d88a

Maybe caused by Bug 509693 ?
Flags: blocking1.9.2?
Confirming it happens on other flash based pages like www.youtube.com also.
Assignee: nobody → roc
Should also note that after you switch tabs, scroll wheel is functional again.
Another symptom is that, on youtube for example, all keyboard input is stolen.
(ctrl+t,ctrl+n, etc do not work)

And as reported in my duplicate, clicking on any text field on the page gives back the focus to the browser. It's just clicking in any non-input control that does not return control(when it should, clicking on the page is a clear sign of wanting control back to the browser).
Is this Flash-only?
On Windows WM_MOUSEWHEEL events are directed to the focused window. A window that isn't handling the WM_MOUSEWHEEL event should call DefWindowProc to forward the message to its parent window, but Flash isn't doing that. However, the real problem here seems to be that clicking outside the Flash plugin isn't giving focus to the browser window.
Yes, the cursor remains in the search field of the embedded movie if you focus the page, should have mentioned that in comment 0. 
The URL is gone for some reason, maybe someone can find a new one?
URL: http://zaplog.nl/zaplog/article/draad...
In nsFocusManager::Blur we have code
    // if an object/plug-in is being blurred, move the system focus to the
    // parent window, otherwise events will still get fired at the plugin.
But when we click outside the plugin, mFocusedContent is null, and the call to SetFocus on our content window doesn't happen.

I assume that Flash calls ::SetFocus on its own window to take focus when we click on it. But how does nsFocusManager know to update mFocusedContent? There are a few ways we could detect that we're losing focus to a plugin, for example by handling WM_KILLFOCUS and observing that the window that's becoming focused belongs to a plugin, and working backwards from that to the plugin element. But we don't do that. We certainly don't get a call to nsFocusManager::Blur. In fact if I focus a text input and then click in the plugin, focus is in the plugin but we still have a blinking caret in the text field since we don't know about the focus change.

I guess this used to work somehow, but I can't see how, so I don't know how any of my changes broke it.

I'll make a build before my changes and see how that works, but Neil, it would be helpful if you could just explain how it's supposed to work :-).
Er, Neil, see comment #8.
The bug is NOT on this page, although it has an embedded movie: http://brabosh.com/2009/10/01/de-ondergang-en-verdrijving-van-de-joden-van-lybie/
URL: http://infidelsunite.typepad.com/coun...
I would assume that this is because the flash in it does not intercept the events. But I think this should be irrelevant to the issue, as the issue is that focus isn't passed back to the browser when clicking anywhere else on the page.
(In reply to comment #8)
> I assume that Flash calls ::SetFocus on its own window to take focus when we
> click on it.
*We* give the focus to the plug-in here by firing NS_PLUGIN_ACTIVATE event.
http://mxr.mozilla.org/mozilla-central/source/modules/plugin/base/src/nsPluginNativeWindowWin.cpp#306
Then nsObjectFrame will catch the event and call the focus manager.
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsObjectFrame.cpp#1832
Ah, thanks! Unfortunately my Windows build is already half-clobbered so I'll have to resume debugging this in the morning.
Ria, are you sure that bug 513299 wasn't in the regression window? That also landed on October 1 and changed the handling of focus events...
Robert - that is possible. The regression window reflects only the time span when the original URL stopped working properly, but the real reason may be further in the past.
Also note: this is not bug 483136 or bug 78414.  Where those two bugs are almost the opposite problem as this: user cannot scroll wheel pages or execute keyboard shortcuts while plugins have hover or focus and the user has to click on the page or reload/switch tabs for it to work again.
This is a regression from bug 508495.
Blocks: 508495
No longer blocks: 509693
The problem is that in bug 508495 we start associating the plugin's widget with an "inner view" which is not directly attached to the nsObjectFrame. Then in nsPresShell::HandleEvent, an NS_PLUGIN_ACTIVATE event can't find a frame for the view, and this is not one of the event types that nsPresShell::HandleEvent will search up the view hierarchy to find a frame for, so it gets dropped on the floor. Adding NS_PLUGIN_ACTIVATE as an event type that it will search up the view hierarchy for fixes the bug.
Attached patch fix (deleted) — — Splinter Review 
This is the fix. It's trivial and safe. I need someone to review it, but I can't find anyone...
I think this is clearly a blocker and deserves to be P1...
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P1
Comment on attachment 404990 [details] [diff] [review]
fix

Mats, can you review this too?
Attachment #404990 - Flags: review?(matspal)
Attachment #404990 - Flags: review?(mozbugz) → review+
Landed the fix on trunk and 1.9.2:
http://hg.mozilla.org/mozilla-central/rev/ccc6dad9010c
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d53ec8e6a079
Leaving open for the test, which I think I have nearly working..
status1.9.2: --- → beta1-fixed
Status: NEW → ASSIGNED
Flags: in-testsuite?
Target Milestone: --- → mozilla1.9.3a1
Fixed with latest trunk nightly.
Attached patch test (deleted) — — Splinter Review 
This patch implements nsWindow::SynthesizeNativeMouseEvent for Windows and adds a test for plugin focus transfer on mouse clicks that uses that facility (which Markus originally added to test some Mac stuff). The test passes on Mac as well. The test won't run on X since SynthesizeNativeMouseEvent isn't implemented there, it should just bail out with a todo.
Attachment #405206 - Flags: review?(jmathies)
Whiteboard: [needs review]
Comment on attachment 405206 [details] [diff] [review]
test

Looks like ce does support that api call, not sure if we run these types of tests though on that platform.
Attachment #405206 - Flags: review?(jmathies) → review+
Alright, I'll remove the #ifdefs from that section.
Whiteboard: [needs review] → [needs landing]
This is next in line for landing, right after smichaud's OJI patches, right?
This was only open for the landing of the test. The actual bug is fixed on trunk and branch. I'll close this to avoid confusion.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Checked in test:
http://hg.mozilla.org/mozilla-central/rev/4c52df6ba5ad
Flags: in-testsuite? → in-testsuite+
Whiteboard: [needs landing]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: