Closed
Bug 75293
Opened 23 years ago
Closed 23 years ago
Plugin continues to play even after leaving page
Categories
(Core Graveyard :: Plug-ins, defect, P2)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9
People
(Reporter: shrir, Assigned: peterlubczynski-bugs)
References
()
Details
Attachments
(1 file)
1.76 KB,
patch
|
Details | Diff | Splinter Review |
Seen on mac trunk 0405 , using Beatnik plugin 2.2 Do not have quicktime installed. Steps: Get the latest beatnik player plugin from beatnik.com (2.2) Go to above url url plays in beatnik player After a while, leave the page and surf to another page Notice that the music continues in the background..plugin does not stop
Assignee | ||
Comment 1•23 years ago
|
||
Sean says this is a case of Stop() not being called for full-page plugins.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.1
Comment 2•23 years ago
|
||
I'm updating my mac build. Will let you know when I know.
Hardware: PC → Macintosh
Comment 3•23 years ago
|
||
I've confirmed that nsIPluginInstance::Stop() is not getting called.
Comment 4•23 years ago
|
||
I think the problem is that PluginViewerImpl::Destroy() in nsPluginViewer.cpp is not implemented. It should probably do something similar to nsObjectFrame::Destroy() in nsObjectFrame.cpp.
Comment 5•23 years ago
|
||
Here's Beatnik specific url testcase (Beatnik is the only plugin that handles rmf content): http://www.beatnik.com/software/documentation/music_object34/music/trance.rmf This will allow you to test with the Beatnik plugin even if Quicktime is installed.
Assignee | ||
Comment 6•23 years ago
|
||
Upping priority. I'll see if I can fix this sooner. Thanks Sean.
Priority: P3 → P2
Target Milestone: mozilla0.9.1 → mozilla0.9
Assignee | ||
Comment 7•23 years ago
|
||
Comment 8•23 years ago
|
||
The patch fixes this particular problem. I'm getting some pretty wierd mouse behavior though - when I get some time I'll file bugs.
Assignee | ||
Comment 9•23 years ago
|
||
Andrei, Can I get a review?
Comment 10•23 years ago
|
||
OK, I guess you coppied this from ObjectFrame, but You can probably change this: + if (doCallSetWindowAfterDestroy) { + inst->Stop(); + inst->Destroy(); + inst->SetWindow(nsnull); + } + else { + inst->SetWindow(nsnull); + inst->Stop(); + inst->Destroy(); + } to this: !doCallSetWindowAfterDestroy ? inst->SetWindow(nsnull) : 0; inst->Stop(); inst->Destroy(); doCallSetWindowAfterDestroy ? inst->SetWindow(nsnull) : 0; not very important though. As we spoke about earlier, having this code duplicate in two places is not good - can it be factored? Should you open a bug on that. Lastly, is the wierd mouse behavior a result of the patch or does it happen with or without the patch? Assuming the latter, and assuming that you get appropriate review, sr=attinasi
Comment 11•23 years ago
|
||
Opened bug 75582 for the mouse problem I commented on.
Assignee | ||
Comment 12•23 years ago
|
||
Bug 75582 is not related to this and I think I know how to fix it. Bug 74457 was opened on refactoring code from nsObjectFrame.cpp and nsPluginViewer.cpp.
Comment 13•23 years ago
|
||
I like this language thing! Also 'traditional' if statement may look easier to read to some. I agree with Marc on factorizing the code but this is definitely not important now. May be we could somehow to write a common base class for both full-page and embedded plugins to inherit from. Task for the future, if any. Another thing. Once you are touching the file. May be I am missing something, but is there any sense to have class declaration in the .cpp file? With or without all this -- ra=av
Assignee | ||
Comment 14•23 years ago
|
||
Patch checked in, marking FIXED. /cvsroot/mozilla/modules/plugin/nglsrc/nsPluginViewer.cpp,v <-- nsPluginViewer.cpp new revision: 1.52; previous revision: 1.51
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 15•23 years ago
|
||
I've always liked the "quickie" conditional statement, especially in this case as, I think, it reads well with the variable name. I've always found braces more confusing unless your IDE "helps" you as CodeWarrior.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•