Closed Bug 920527 Opened 11 years ago Closed 3 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: 3 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: