Created attachment 393054 [details] testcase ###!!! ASSERTION: Configured widget is not a child of the right widget: 'aConfiguration.mChild->GetParent() == aExpectedParent', file /Users/jruderman/central/widget/src/cocoa/nsChildView.mm, line 1695 The function containing this assertion was introduced in rev 8d7d268bd181: "Bug 339548. Part 3: Introduce native widget clip region API."
Assignee: nobody → roc
The problem here is that scrolling in a popup triggers computation of the plugin geometry which we then try to apply as the "configurations" parameter of the scroll, although we're scrolling a widget which is not the parent of the plugin. I think GetPluginGeometryUpdates needs to only compute geometry for plugins whose toplevel widget is the same as the toplevel widget for aChangedSubtree.
Actually we need to be more strict than that, GetPluginGeometryUpdates assumes that the plugin will be relative to the rootprescontext's widget. We actually need to disable plugins in popups.
Created attachment 394191 [details] [diff] [review] fix This patch disables the rendering of windowed plugins in popups on non-Mac platforms. -- nsRootPresContext::UpdatePluginGeometry should just use its root widget as the parent for the ConfigureChildren operation. All plugin widgets should be children of that widget. We want to get the assertion from ConfigureChildren if that's not the case. -- Scrollframes should only update plugin geometry if our scroll operation is happening in the main window. If it's happening in a popup, plugins cannot be affected and we don't want the popup's widget to touch them. -- In nsObjectFrame::CreateWidget, just leave the plugin window hidden if there is a frame ancestor with a widget that's not the root. On Windows and GTK this will prevent windowed plugins from being visible in popups. On Mac the plugin will still work normally since painting and event handling don't rely on the plugin's widget. But this is probably good since windowless plugins will still work on all platforms. We do actually need to hide the widget on Mac, otherwise Cocoa finds it in the main window and paints it there.
Attachment #394191 - Flags: review?(dbaron)
Whiteboard: [needs review]
Attachment #394191 - Flags: review?(dbaron) → review+
Comment on attachment 394191 [details] [diff] [review] fix > nsRootPresContext::UpdatePluginGeometry(nsIFrame* aChangedSubtree) >- nsIWidget* widget = configurations.mChild->GetParent(); >- NS_ASSERTION(widget, "Plugin must have a parent"); >+ nsIWidget* widget = FrameManager()->GetRootFrame()->GetWindow(); >+ NS_ASSERTION(widget, "Plugins must have a parent window"); Do you want to assert that the old value of |widget| and the new value of |widget| here are the same thing? Or is that not the case? r=dbaron
I'm putting this in my tree for try-server shortly and then (presumably) checkin later.
(In reply to comment #3) > This patch disables the rendering of windowed plugins in popups on non-Mac > platforms. ... > work on all platforms. We do actually need to hide the widget on Mac, otherwise > Cocoa finds it in the main window and paints it there. I'm finding these two statements a little contradictory while trying to write a commit message, though.
So I'm also curious what our story on platform consistency is here? Are we intentionally making behavior different across platforms? Is that a problem? (So I'm not landing this right now.)
The patch makes the widget for a plugin in a popup always be hidden. On Windows and X, this makes windowed plugins in popups completely invisible, whereas before the patch they appear in the main window in the wrong place (or worse). Windowless plugins should not be affected; AFAIK they worked before and still do. On Mac plugins in popups display once in the popup but at the same time in the main window (although the main window version probably doesn't repaint properly). With this patch the version in the main window disappears but it's still visible in the popup as expected. In summary, before the patch, windowed plugins in popups are broken in various platform-specific ways on trunk, and with the patch they're invisible on Windows and X and work on Mac. However, this is really just another manifestation of the limitations of windowed plugins, which Mac plugins don't have.
Whiteboard: [needs review] → [needs landing]
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 192 approval]
Attachment #394191 - Flags: approval1.9.2?
Attachment #394191 - Flags: approval1.9.2? → approval1.9.2+
Whiteboard: [needs 192 approval] → [needs 192 landing]
status1.9.2: --- → final-fixed
Whiteboard: [needs 192 landing]
Depends on: 529263
The testcase for this bug touches the network, which caused the crashtest suite to fail, and the tree to be closed for several hours today. I filed bug 616085 to disable the test.
You need to log in before you can comment on or make changes to this bug.