Closed Bug 529263 Opened 12 years ago Closed 12 years ago

Flash contents in the sidebar are not rendered properly

Categories

(Core :: Plug-ins, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta1+
blocking1.9.2 --- .2+
status1.9.2 --- .2-fixed

People

(Reporter: kohei.yoshino, Assigned: roc)

References

Details

(Keywords: regression, verified1.9.2)

Attachments

(2 files, 1 obsolete file)

How to reproduce:

1. Bookmark http://www.adobe.com/ (or an arbitrary Flash site)
2. Open Library, select the bookmark and check "Load this bookmark in the sidebar".
3. Click the bookmark to load in the sidebar

It doesn't work with Firefox 3.6 and trunk while it works with 3.5.x and Windows builds.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20091116 Minefield/3.7a1pre

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; ja-JP-mac; rv:1.9.2b3) Gecko/20091115 Firefox/3.6b3
I have 10.0 Shockwave Flash 10.0 r32, the latest version.

This build shows a blank rectangle overlay on the main content area as large as the Flash content but the playback goes smoothly:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20091001 Minefield/3.7a1pre

This build doesn't work well:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20091002 Minefield/3.7a1pre

The regression range?

http://hg.mozilla.org/comm-central/pushloghtml?startdate=2009-09-30&enddate=2009-10-02
Still reproducible with the latest trunk.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20100104 Minefield/3.7a1pre

This bug affects the J-WAVE Toolbar add-on which has a sidebar web panel with embedded Flash contents:

https://addons.mozilla.org/firefox/addon/48910
Assignee: nobody → roc
Flags: wanted-next+
This seems like a pretty big regression. But no traction for 3.6?
This bug seems be reproducible not only on Mac but also Windows/Linux.
OS: Mac OS X → All
Hardware: x86 → All
I'm upping the severity on this one. This is a major regression. It's breaking quite a few of our extensions.
Severity: normal → major
The sidebar appears to be a <xul:browser> which is *not* type="content". Are you trying to load a plugin from chrome content? This and bug 534355 are probably the same bug.

The bug here seems to be that the sidebar should be type="content". I don't know what that would break.
Depends on: 534355
> The sidebar appears to be a <xul:browser> which is *not* type="content".

Uh.... it's not?  Why not?  It should be.  It was last I checked, for things other than bookmarks and whatnot!  Was I just confused?  If not, what changed and when?
At least in my case, I have my own sidebar.xul in which I put a <browser type="content"> and the flash doesn't render.

The plugin loads (if I right click, I see the flash menu), it just won't load content.
Michael, it sounds like you might be seeing a different bug.  Can you please file it, along with whatever xul file should be run with -chrome to reproduce it?
(In reply to comment #7)
> The sidebar appears to be a <xul:browser> which is *not* type="content".

How did you observe this? STR from comment 0 result in a sidebar with a <browser type="content"> for me (verified with DOM inspector). We hardcode that here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/web-panels.xul#95
The sidebar panel of the J-WAVE Toolbar (above-mentioned) has <browser> with type="content".

<browser id="jwave-sidebar-frame" src="http://www.j-wave.co.jp/locmag/tlbar_v.htm" type="content" style="width: 100%" flex="1" />
Blocks: 542289
I've debugged and this is the same problem we have. And the sidebar IS type=content.

The problem is simply that flash content is not working in the sidebar for some reason.

Here's an example of one of our sidebars:

http://www.thendl.com/NDL-sidebar/sidebar.asp

If you add it as a book mark and open it in the sidebar, it says "Adobe Flash Player required" This is the exact same problem we have with our custom sidebar.

Looking at the Browser DOM node in the bookmark case (web panels)

<browser xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" id="web-panels-browser" persist="cachedurl" type="content" flex="1" context="contentAreaContextMenu" tooltip="aHTMLTooltip" onclick="return window.parent.contentAreaClick(event, true);" cachedurl="http://www.thendl.com/NDL-sidebar/sidebar.asp"/>

It's type="content"

So there was a MAJOR regression displaying flash in the sidebar.

This all works in Firefox 3.5.

Does anyone even have a clue as to why this regressed?

This is a huge problem for us.
blocking2.0: --- → ?
This is a regression from bug 508908. In particular, this test:

    if (parentWidget == GetWindow()) {

in nsObjectFrame::CreateWidget tests false for the plug-in in the sidebar.
Blocks: 508908
Well, in this case |rpc| is the toplevel Firefox window prescontext, but GetWindow() returns the widget corresponding to the viewport frame of the sidebar HTML document (or at least stepping into GetWindow has this at the point when we're about to do |return v->GetWidget();| I get:

(gdb) p ((nsIFrame*)v->GetClientData())->PresContext()->mShell->mDocument->mDocumentURI.mRawPtr->mSpec.mData
$26 = 0x20f781d8 "http://www.thendl.com/NDL-sidebar/sidebar.asp"

So it looks to me like the root prescontext is wrong in the sidebar case for some reason...
And in particular, when we're setting up that sidebar.asp document viewer and DocumentViewerImpl::InitInternal calls FindContainerView, it finds the inner view of the web-panels.xul subdoc frame.  And in particular, mParentWidet is not null, so |containerView| ends up non-null as well.  But web-panels.xul is itself loaded in a subframe of browser.xul, so we hit this code in FindContainerView:

2389   // see if the containerView has already been hooked into a foreign view manager hierarchy
2390   // if it has, then we have to hook into the hierarchy too otherwise bad things will happen.
2391   nsIViewManager* containerVM = containerView->GetViewManager();
2392   nsIView* pView = containerView;
2393   do {
2394     pView = pView->GetParent();
2395   } while (pView && pView->GetViewManager() == containerVM);
2396   if (pView)
2397     return containerView;

and hook the sidebar into the chrome viewmanager hierarchy, even though it has type="content".  But the type="content" subframe still has a widget at its root, so we get the observed behavior...

roc, do we really need this "see if ..." codepath at this point?
No, I don't think we do.
So is the fix to remove all that code?

Any chance we could put that fix on a try server and I could see if it fixes the other flash sidebar problems I'm seeing?
I pushed that change to the try server. I believe builds will show up here:
https://build.mozilla.org/tryserver-builds/rocallahan@mozilla.com-try-a141be2ee10f/
although there's nothing there yet.
I've verified that this build all the flash sidebar problems I was seeing.
That sentence is missing a verb....  Is the missing verb the present form of "to have" or the present form of "to fix"?
Doh :)

It fixes all of my flash issues in the sidebar.
(In reply to comment #20)
> https://build.mozilla.org/tryserver-builds/rocallahan@mozilla.com-try-a141be2ee10f/

J-WAVE Toolbar's sidebar panel works with this build.
Attached patch fixSplinter Review
Attachment #426754 - Flags: review?(bzbarsky)
Comment on attachment 426754 [details] [diff] [review]
fix

Take out the three lines of comment (two text, one blank) right after the code you removed, and r=me
Attachment #426754 - Flags: review?(bzbarsky) → review+
Whiteboard: [needs landing]
blocking1.9.2: --- → ?
The reason I'm requesting blocking is because this was a major regression in Firefox 3.6.
Agreed; please put the patch up for 1.9.2.2 approval when it's branch-safe and after it's baked a cycle on trunk.

Also: I would really, really, really appreciate a test for this to ensure we don't break it again.
blocking1.9.2: ? → .2+
blocking2.0: ? → beta1
Is there someone that can check this in?
http://hg.mozilla.org/mozilla-central/rev/efe73e4926cd
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Comment on attachment 426754 [details] [diff] [review]
fix

Fixes a significant regression for extensions in 3.6.
Attachment #426754 - Flags: approval1.9.2.2?
Comment on attachment 426754 [details] [diff] [review]
fix

a1922=beltzner, with another plea for tests
Attachment #426754 - Flags: approval1.9.2.2? → approval1.9.2.2+
Whiteboard: [needs 192 landing]
OK, now I've just got a plea to land it.
Duplicate of this bug: 552059
Verified for 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.2) Gecko/20100316 Firefox/3.6.2 using the testcase in comment 0.
Keywords: verified1.9.2
Duplicate of this bug: 544694
If this was fixed in February, why hasn't it been included in a release if it's such a critical issue??

I ask because I'm using version 3.6.8 and the problem persists.
It was fixed in 3.6.2 as far as I can tell.

What are your exact steps to reproduce it and what operating system are you using?
Screenshot shows 2 versions of Youtube:  1 in an iframe, 1 in a browser
Attachment #464831 - Attachment is obsolete: true
Clarification:  The <iframe> tags are used in the top frame and the <browser> tags were used in the bottom.  As you can see, there's mush where the flash video should be in the top frame while the video displays properly in the browser frame.

On another note, I don't want to use <browser> tags because (if you look closely), the browser overlaps the edge of the sidebar.  There's a thin grey line on the edge of the iframe but there's none at the edge of the browser.  The overlapping makes it so that the user cannot expand the sidebar to the right.  The option just isn't there!
Can you please file a separate bug on that issue, with clear steps to reproduce?  If you're using a <browser>, then you're not using what this bug means by "sidebar" (which are all content docshells and can't have <browser>s in them).
Our sidebar using iframe tag with type='content' displays just fine in version 3.6.8 running on Windows 7 with Flash 10.1.82.76 installed.
FYI, wmode we used is 'window'. Don't remember if other modes worked or not unfortunately but do remember having issues with other iframe types.

'content' iframe type sandboxes content so it's a hassle to work with but you can use events and DOM elements to bridge over sandbox security.
You need to log in before you can comment on or make changes to this bug.