I have seen this in a few bugs, logging a new one for clarity sakes. seen on 0618 brnch NT, make sure you have realplayer installed. steps: go to the above page Click on "Listen Live" under 'On Air Now' 'Change player' to realplayer inside the new window that pops up.. observe that the audio feed jsut does not start

and the embed or object has the parameter set to automatically start?

yeah 'autostart =1' or 'true' is set in the source...checked it.

assigning to peterl

testcase shows that calling the realplayer scripting function from within the document's onLoad() does not work, calling it thru submit works. Thx, Peter !

This is a serious problem -- calling any scripting function on a plugin during the onLoad handler will fail. The reason for this is because plugins are tied to the frame model rather than content/DOM. We do not create the plugin until we reflow the layout frame. The onLoad handler fires before we have a chance to create the plugin instance in the object frame. One solution to this problem is outlined in bug 90268 where we would move plugin creation to content (like jst recently did with iframes). But fixing that is not going to be easy. I wonder if there is hack we can come up with that can be done in the meantime to get C|Net radio working? btw, thanks for testcase Shrir!

Update: This only happens in a "nested" tag situation. Removing the outer OBJECT tag allows for scripting from onLoad. For debugging, I'm setting a breakpoint in |nsDocShell::EndPageLoad| which calls the onLoad handler. Here is the root problem as to why it only fails in a nesting situation: During Reflow, we call |CantRenderReplaceElement| on the outer ActiveX OBJECT tag. This is an asynchronous function (via PLEvent) that causes the CSSFrameConstructor to make frames for the contents of the non-renderable outer OBJECT tag. It appears in |PresShell::ProcessReflowCommands| we call |DoneRemovingReflowCommands| which causes the onLoad handler to fire while we still have a CantRenderReplaceElement event in the queue. I'm going to see if I can delay the onLoad handler if there is a pending CantRenderReplaceElement.

Here's a hack that seems to get C|Net Radio playing. It delays the calling of the onLoad handler by means of adding these "dummy" requests in the pres shell. When we call |CantRenderReplaceElement|, the patch adds two requests to the load group. I still need to run more tests to ensure it doesn't break anything other combinations of content.

Here's a patch I feel better about. Instead of re-using the pres shell's dummy layout request, I give each CantRenderReplaceElement it's own. Using the constructor/destructor of the PLEvent is a much better way to ensure we only add and remove once from the load group per event which was a problem with the last patch. So basically with this patch while we have a pending CantRenderReplaceElement PLEvent, we'll have a dummy request in the load group to delay the onLoad handler.

doug/serge: we are trying to wrap up 1.0.1. where are we on getting reviews for the latest patch?

my apologies ... my last comment was meant for peterl. chalk it up to a copy and paste error.

I reviewed attachment(id=91806) but I suggested a different approach. The current patch creates a dummy request instance for every replaced element. This could be expensive on pages with lots of input elements. I suggested an alternative approach of keeping a count of the number of replaced elements that have not been loaded. The dummy request would not be removed until all of the replaced elements had loaded.

New in this patch: * keeps a count instead of allocating memory as suggested * ensures we've attempted to remove the request * isolated to object frame (applet/object/embed tags)

I think it would be a little easier to undertand and cleaner if the counter was expanded to indicate whether the document has completed loading regardless of whether it was waiting for the normal document to load (i.e no more reflows pending) or replaced elements were still in the process of loading. I think this could be accomplished by doing the following: Change method name and signature of HoldLoadGroupRequestCount(nsIPresShell::eHold_Increase) to something like the following: NotifyLoadCompleted(PRBool aLoadIsComplete) if (aLoadIsComplete) than decrement the counter if (!aLoadIsComplete) than increment the counter Change mHoldLoadGroupRequestCount to mLoadingCount Get rid of having a separate flag for mAttemptedToRemove. Instead the NotifyLoadCompleted(PR_FALSE) would be called from AddDummyLayoutRequest. This would set the mLoadingCount to 1. Modify DoneRemovingReflowCommands to call NotifyLoadCompleted(PR_TRUE). This would set the counter back to 0 and trigger a call to Remove the DummyLoadRequest . Some code snippets: FrameManager::DestroyPLEvent(CantRenderReplacedElementEvent* aEvent) { ... if (shell) shell->NotifyLoadCompleted(PR_TRUE); delete aEvent; } CantRenderReplacedElementEvent::CantRenderReplacedElementEvent(FrameManager* aFrameManager, nsIFrame* aFrame, nsIPresShell* aPresShell) { ... if (nsLayoutAtoms::objectFrame == frameType) { mPresShell = getter_AddRefs(NS_GetWeakReference(aPresShell)); aPresShell->NotifyLoadCompleted(PR_FALSE); } NS_IMETHODIMP PresShell::NotifyLoadCompleted(PRBool aLoadComplete) { if (aLoadComplete) { mLoadingCount--; if (mLoadingCount <= 0) { RemoveDummyLayoutRequest(); } } else { mLoadingCount++; } return NS_OK } PresShell::AddDummyLayoutRequest(void) { ... rv = loadGroup->AddRequest(mDummyLayoutRequest, nsnull); NotifyLoadCompleted(PR_FALSE); ... } void PresShell::DoneRemovingReflowCommands() { if (mRCCreatedDuringLoad == 0 && mDummyLayoutRequest && !mIsReflowing) { NotifyLoadCompleted(PR_TRUE); // RemoveDummyLayoutRequest(); <== This will be called from NotifyLoadCompleted when the count drops to 0} }

update patch to Kevin comments. However, I had a problem in that |FrameManager::ReplaceFrame| eventually caused |DoneRemovingReflowCommands| to be called and the onLoad would fire before the new frame was created. So, I use a second lock in |nsCSSFrameConstructor:CantRenderReplacedElement| which did the trick. It seems that this code is called quite frequently, on at least every page. It would be good to do some more testing, especially if something were attached to the onLoad handler like in a cycle-through test.

After speaking with Kevin, patch v.2 with a few modifications would seem safest for the branch while a better solution using the approach in patch v.4 could be explored for the trunk. Here's basically the same patch as v.2 except we will only add a request to the load group if we are doing a CantRenderReplacedElementEvent on an objectFrame. The request is added in the constructor and removed in the destructor of the event.

Comment on attachment 92301 [details] [diff] [review] patch v.5 r=kmcclusk@netscape.com

Comment on attachment 92301 [details] [diff] [review] patch v.5 sr=dveditz

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending drivers' approval. pls check this in asap, then replace "mozilla1.0.1" with "fixed1.0.1". thanks!

Comment on attachment 92301 [details] [diff] [review] patch v.5 a=chofmann for 1.0.1. add the fixed1.0.1 keyword after checking into the branch.

patch v.5 checked in on branch

shrir: pls verify this as fixed on the 1.0 branch. thanks!

Jaime, there's no branch build out yet. Will verify this and check for any regressions as soon as the build comes out.

works like a charm, verified on 0723 branch . Also tested for regressions, none seen. Verified ! Is this not on the trunk yet ? what to do with the bug resolution?

I'm planning to land on trunk after getting drivers approval and then open a new bug on expanding this fix to all types of frames.

it seems like we should check in the patch for bug #106253 too. It's just a different facet of the same problem -- that plugins aren't playing well with loadgroups... -- rick

looks like the patch for bug #106253 was backed out of 1.0 in May.

C|Net defaults the SRC attribute to "" so I don't think the patch from bug 106253 would not have any effect here. The patch in bug 106253 was backed out of the branch because it caused problems with Real described by Rick in this comment: http://bugscape.mcom.com/show_bug.cgi?id=14295#c49

a=asa (on behalf of drivers) for checkin to 1.1

patch in trunk, marking FIXED.

verif on 1014 trunk , this works now.