Summary: Embedded YouTube video's controls render as clipped I've been seeing this for at least the past 3 days, I think. STR: 1. Load http://lexusenthusiast.com/2009/07/29/twin-turbo-lexus-is-f-video/ 2. Look at the embedded YouTube video Expected Results: The embedded YouTube video should look as it does elsewhere: controls rendered intact. Actual Results: The video-controls bar is rendered clipped.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090804 Minefield/3.6a1pre Happens on Windows as well. With Shiretoko it looks normal.

We need a regression range.

Regressed between the builds 090722 and 090723: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=02f8bf10f441&tochange=684dadc0726e Eventually bug 502447? I more granular window would be helpful.

My fault. Regressed between 0721 and 0722: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f2a58ffcd00c&tochange=02f8bf10f441 Looks like it is one of the checkins from Robert.

Regression from bug 339548.

The EMBED has borders and padding. I guess I need to fix borders and padding on plugins properly now instead of just ignoring them.

This is a 200x200px flash object with a 10px border. It should render as a solid 200x200px green square. (It would be easy enough to modify this testcase to work with the test plugin, but I figured this was more usable as a testcase for most people.)

David, if you want someone else to review this, just say the word. This patch makes borders and padding work on plugin elements. The plugin fills the content-box. -- Add an mInnerView for nsObjectFrames to hold the plugin. We need this because the plugin's widget can dispatch events through its associated view, and the view needs to be aligned to the content-box. Adding views sucks, but for now it seems necessary. -- Size and position the plugin widget based on the content-box. -- Windowless plugin painting needs to set things up to draw into the content-box. -- When we translate event coordinates to send to the windowless plugin, we need to subtract off the border+padding.

The nsViewManager change is to avoid an assert when you set bounds with nonzero origin when creating a view.

Additional fixes to use the right size when printing a plugin with borders or padding. I'm also removing two chunks of printing code: Unix code that no longer works because we don't support EPS (and is already #if 0), and fallback code that's used if !defined(XP_MACOSX) && !defined(XP_UNIX) && !defined(XP_WIN) && !defined(XP_OS2) --- i.e. it's never really used.

(In reply to comment #8) > -- Add an mInnerView for nsObjectFrames to hold the plugin. We need this > because the plugin's widget can dispatch events through its associated view, > and the view needs to be aligned to the content-box. Adding views sucks, but > for now it seems necessary. Can we remove nsObjectFrame::NeedsView to compensate? Also, while investigating that, I think it looks like nsIFrame::CreateWidgetForView and nsObjectFrame::CreateWidgetForView are completely unused; only nsMenuPopupFrame::CreateWidgetForView appears to still be called. The old calls in nsFrame.cpp and nsObjectFrame.cpp seem to have been removed since 1.9.1; I'd suspect relatively recently (compositor phase 1?).

Also, might be good to test either an rgba(), dotted, or transparent border in addition to your current tests so that you're testing what's drawn in the border area too. Though you are testing padding, so it's probably ok.

(In reply to comment #11) > (In reply to comment #8) > > -- Add an mInnerView for nsObjectFrames to hold the plugin. We need this > > because the plugin's widget can dispatch events through its associated view, > > and the view needs to be aligned to the content-box. Adding views sucks, but > > for now it seems necessary. > > Can we remove nsObjectFrame::NeedsView to compensate? No, our view-management code would fail to find mInnerView and so it wouldn't get moved due to ancestor frame reflow etc. > Also, while investigating that, I think it looks like > nsIFrame::CreateWidgetForView and nsObjectFrame::CreateWidgetForView are > completely unused; only nsMenuPopupFrame::CreateWidgetForView appears to still > be called. The old calls in nsFrame.cpp and nsObjectFrame.cpp seem to have > been removed since 1.9.1; I'd suspect relatively recently (compositor phase > 1?). I just noticed this myself! I'll remove them. (In reply to comment #12) > Also, might be good to test either an rgba(), dotted, or transparent border in > addition to your current tests so that you're testing what's drawn in the > border area too. Though you are testing padding, so it's probably ok. I'll just make my borders in the existing tests dotted, that should cover it.

Comment on attachment 399176 [details] [diff] [review] fix Is there a reason nsObjectFrame::CreateWidget sets the size of the frame's view? Should you do the same for the inner view? >+ origin += GetContentRect().TopLeft() - GetPosition(); It's a bit less work to just do: nsMargin bp = GetUsedBorderAndPadding(); origin += nsPoint(bp.top, bp.left); (Maybe even add a TopLeft() helper to nsMargin?) Thought that does assume nsObjectFrame::GetSkipSides always returns 0; I think that's a fair assumption but it's worth commenting. Same in PaintPrintPlugin. and nsPluginInstanceOwner::ProcessEventX11Composited (three times). Maybe make a function for it? r=dbaron

I'll make a function for it. > Is there a reason nsObjectFrame::CreateWidget sets the size of the frame's > view? Should you do the same for the inner view? To make sure that the widget gets created with the right initial geometry. I guess I should do that for the inner view now.

(In reply to comment #15) > To make sure that the widget gets created with the right initial geometry. I > guess I should do that for the inner view now. Oops, I already do when it's created.

http://hg.mozilla.org/mozilla-central/rev/29e974a88602 http://hg.mozilla.org/mozilla-central/rev/ac37ce54ddb0 Removing CreateWidgetAndView: http://hg.mozilla.org/mozilla-central/rev/5cc6bebd0dd9

Roc, when I want to print the page given by comment 0 the embedded video renders too small and doesn't fit into the surrounding frame on OS X. Is this a flash problem or our fault?

I don't know, file a new bug for that.

Verified FIXED on trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091006 Minefield/3.7a1pre.

(In reply to comment #18) > Roc, when I want to print the page given by comment 0 the embedded video > renders too small and doesn't fit into the surrounding frame on OS X. Is this a > flash problem or our fault? Steven, is this the same thing as what we have seen on bug 191046? I know I stumble over it again and again. Is a new bug needed or can't we fix that?

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/3983225cd60c http://hg.mozilla.org/releases/mozilla-1.9.2/rev/748e664edc71

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20091007 Namoroka/3.6b1pre ID:20091007034618