Orphan VIDEO element are Zombies and can be a unexpected annoyance when closing a tab. ie, orphan VIDEO element which was dynamically created using document.createElement("video"); Or something which was remove videoParent.removeChild(videoNode); Or something which was disappeared by videoParent.innerHTML=""; Or something which was wiped out by document.write(); BTW What all should be the capabilities of an orphan VIDEO element * Should we allow videoNode.play(); on them? * Can we adopt video node from one document to other while it is still playing? That will be a nice to have feature to solve bug 449539. Steps to reproduce bug Case 1 1. open attached zombies_1.html 2. you will hear something started playing 3. keep playing few seconds 4. Click "Navigate away to Google" link Result: video continues to play i think till the end of it Expected: Video stop immediately Case 2 1. open attached zombies_1.html 2. you will hear something started playing 3. keep playing few seconds till you hear sound of engine/motor 4. press reload button to refresh page Result: both video plays ie, the first one with sound of engine/motor i think at some time first one ends Expected: the first Video stop immediately user should hear only current Video Case 3 1. open attached zombies_1.html 2. you will hear something started playing 3. keep playing few seconds 4. Close tab Result: video continues to play for few seconds Expected: Video stop immediately Repeat same with attached zombie_2.html, zombie_3.html, zombie_4.html on zombie_4.html the actual results are slightly different FAIL Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090228 Minefield/3.2a1pre FAIL Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090223 Shiretoko/3.1b3pre

This might be annoying behaviour, but I'm not convinced that it's incorrect behaviour. According to the HTML5 spec: "Media elements that are potentially playing while not in a Document must not play any video, but should play any audio component. Media elements must not stop playing just because all references to them have been removed; only once a media element to which no references exist has reached a point where no further audio remains to be played for that element (e.g. because the element is paused, or because the end of the clip has been reached, or because its playbackRate is 0.0) may the element be garbage collected."

(In reply to comment #4) > "Media elements that are potentially playing while not in a Document must not > play any video, but should play any audio component. Good, I can agree with spec. So that part of issue we are doing as per spec. But still it should not do what we fixed in bug 470636 ie, at present audio doesn't stop after navigating away from the page. And Expected Result:- Audio should stop immediately after we navigate away from the page.

Warning !!! You will be Rick Roll'D

Bug 470636 is different, the media element is in the document in that case. In this case, it's not, and the part of the spec I quoted applies. Navigating away destroys the document, but the media element is not part of the document, so should continue to play per the spec. I don't think there's a bug here, so resolving as invalid.

(In reply to comment #7) > Bug 470636 is different, the media element is in the document in that case. The effect is the same, though. Don't you agree that a site being able to cause audio to continue playing after you've navigated away from it is a bad thing? It seems to me that the intent of that part of the spec is to ensure that video/audio elements can be used without being inserted into a document. I don't see any valid use cases for them outliving the associated document, or playing while the document is in bfcache, so I think the spec (and our implementation) should be fixed to include that limitation.

This rather long thread goes into the history of how the spec ended up where it is now: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2007-October/012798.html According to the thread it looks like audio objects outside of the document should not be garbage collected until the browsing context goes away.

Reading the spec in the context of that mailing list discussion convinces me that we should stop playing when the document becomes inactive. I do think that the spec could be clearer on this issue, but this passage seems to cover this case: "Note: If the media element's Document stops being an active document, then the playback will stop until the document is active again."

I think we actually will stop playing when the document is destroyed via cycle collection. The problem here is that our PresShell::Freeze code iterates through the nodes in the document to decide which ones to freeze ... but that misses the nodes that aren't in the document. I think we need to add a list of a document's media elements to the document, so we can accurately freeze them all.

Make each document store the set of elements that it owns that may need to be frozen --- plugins and media elements. The test code is a little tricky. Hopefully the README file explains what we're doing here. It's important to be able to ignore the destruction of plugin instances that were created before we started tracking them --- because of asynchronous plugin destruction there may be instances hanging around when the test starts that get destroyed later. This is especially a problem when you reload the testcase.

Comment on attachment 375057 [details] [diff] [review] fix Isn't this patch missing code to register plugin elements as freezable elements?

Yes! My test was wrong, the iframe'd plugin was not going into bfcache at all because we don't bfcache iframes by default. So I've revised the test to use a separate window, and the test failed with the old patch. Here's the fixed patch and test.

roc, looks like that last attachment is a fix for another bug...

oops

Comment on attachment 376342 [details] [diff] [review] fix v2 r+sr=jst

http://hg.mozilla.org/mozilla-central/rev/fdd555ade242 There's no obvious way to test the actual sound output, but there is a test that plugins are actually stopped.

Tanx Roc... Tested on both Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090509 Minefield/3.6a1pre Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090509 Minefield/3.6a1pre a PASS for case 1/2/3 on zombie_1/2/3/4.html and RickRolled.html PS: instead of stopping immediately, it will be after few (say 3) seconds

I disabled the test because it was causing focus problems: baf06abf0af4 (In reply to comment #19) > PS: instead of stopping immediately, it will be after few (say 3) seconds That's probably just the time required to drain the audio buffers. 3 seconds sounds a bit long though.

(In reply to comment #20) > 3 seconds sounds a bit long though. on my PC usually it is a minimum of 3sec, Some times it even went to 10sec, I think we can live that for now.

The test doesn't work on 1.9.1 because the test plugin stuff I need is not there, so I've removed that from the patch. One thing I'm not sure about is the change to nsIDocument. Are we still allowed to rev its IID on the 1.9.1 branch? Or can we avoid revving the IID since I'm not changing any virtual methods?

I think I should just land this without revving the nsIDocument IID.

I've backed this out from 1.9.1 as it was causing crashes on the linux reftests and crashtests: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1242803401.1242808896.30194.gz&fulltext=1

The landing was http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8d98bfd5645a The backout was http://hg.mozilla.org/releases/mozilla-1.9.1/rev/acd2d4638228 (thanks Dave) We crashed after this test: REFTEST TEST-PASS | file:///builds/slave/mozilla-1.9.1-linux-unittest/build/layout/reftests/bugs/84400-1.html | Thread 0 (crashed) 0 libxul.so!FreezeElement(nsIContent*, void*) [nsISupportsUtils.h : 203 + 0x3] eip = 0x40368b51 esp = 0xbfe372ec ebp = 0xbfe37308 ebx = 0x40dbf5a0 esi = 0x429e18f0 edi = 0x42958db4 eax = 0x00000000 ecx = 0x429e18f0 edx = 0xbfe372f8 efl = 0x00010282 1 libxul.so!EnumerateFreezables(nsPtrHashKey<nsIContent>*, void*) [nsDocument.cpp:c7babba8df56 : 7580 + 0xa] eip = 0x404b1936 esp = 0xbfe37310 ebp = 0xbfe37328

When merging to branch I turned do_QueryFrame into CallQueryInterface, but the former is null safe while the latter is not, which lead to this crash. So I added an "if (frame)", tests were OK, and I relanded. http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8c7f6dd7990b

Verified FIXED on latest 1.9.1 branch and trunk, as well tested in b99 Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090608 Minefield/3.6a1pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b99) Gecko/20090604 Firefox/3.5b99

(In reply to comment #22) > Created an attachment (id=377621) [details] > 1.9.1 patch > > The test doesn't work on 1.9.1 because the test plugin stuff I need is not > there, so I've removed that from the patch. FYI, there's a blanket approval for landing tests and test harness changes on branches, so feel free to land test plugin changes on the branch.