Closed Bug 920527 Opened 9 years ago Closed 2 years ago

Improve book-keeping and add/remove events for plugins

Categories

(Core Graveyard :: Plug-ins, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: gfritzsche, Unassigned)

References

Details

Attachments

(1 file)

(John Schoenick [:johns] from bug 916542, comment #8)
> (In reply to Georg Fritzsche [:gfritzsche] from bug 916542, comment #4)
> > (1) In |nsObjectLoadingContent::BindToTree()| we currently always call
> > |aDocument->AddPlugin()| [1] without checking if the object type is actually
> > a plugin.
> > (2) This means that non-plugin object elements show up in
> > nsIDOMWindowUtils.plugins [2].
> 
> That list has always really been a "plugin-capable tags" list. It wouldn't
> be too hard to make it only contain tags configured as a plugin, but we'd
> need to audit its users.
> 
> > (3) Those things surfaced here because the "PluginRemoved" event also fires
> > without checking whether the object is a plugin.
> 
> As I noted in bug 889788 comment 6 when PluginRemoved was added, it doesn't
> catch every case and can fire when no plugins were really removed from the
> page, among other things. The proper fix here is to setup a pair of
> PluginAdded PluginRemoved events that actually fire when we change type
> to/from eType_Plugin, and not enumerate every plugin on the page every time
> the doorhanger appears, or we're going to be chasing edge cases for a while.
> 
> > I'm assuming that (1) and probably (3) are bugs John?
> > Does it sound correct to fix (1) by calling
> > nsIDocument::AddPlugin()/RemovePlugin():
> > * in BindToTree()/UnbindFromTree() only when mType==eType_Plugin
> > * whenever mType goes from or to eType_Plugin and the element is bound to a
> > tree
> > ... plus fire events to trigger the CtP frontend code.
> 
> Assuming other users of document.plugins don't mind, yes - but there are
> numerous ways to spawn/stop a plugin, LoadObject/UnloadObject would be the
> proper place, or InstantiatePluginInstance/StopPluginInstance if we only
> want "live" plugins.
Test-case from bug 922349 that should be adressed properly too:
See bug 922349, comment 6 to 9.
Taking this for later, although i'd of course be happy to hand it over if someone else can get to this sooner than me.
Assignee: nobody → georg.fritzsche
Depends on: 922349
Open question for me: would it make sense to fire "PluginRemoved" always at the top-document? Or are there any use-cases for which we might need to know the subdocument it got removed from?
We really shouldn't bother with PluginRemoved. We don't want it long-term. Once a plugin has been used in a document, the plugin doorhanger should include it permanently.
The permanently-showing doorhanger will be handled by bug 926605.
Assignee: georg.fritzsche → nobody
Flags: firefox-backlog?
Flags: firefox-backlog? → firefox-backlog+
Flags: firefox-backlog+ → firefox-backlog?
Flags: firefox-backlog? → firefox-backlog+
Resolving as wont fix, plugin support deprecated in Firefox 85.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.