Improve book-keeping and add/remove events for plugins

NEW
Unassigned

Status

()

Core
Plug-ins
P3
normal
5 years ago
3 years ago

People

(Reporter: gfritzsche, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
(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.
(Reporter)

Comment 1

5 years ago
Created attachment 812786 [details]
Test-case: subdocument/frame removal with plugin content

Test-case from bug 922349 that should be adressed properly too:
See bug 922349, comment 6 to 9.
(Reporter)

Comment 2

5 years ago
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
(Reporter)

Comment 3

5 years ago
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?

Comment 4

5 years ago
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.
(Reporter)

Comment 5

5 years ago
The permanently-showing doorhanger will be handled by bug 926605.
(Reporter)

Updated

4 years ago
Assignee: georg.fritzsche → nobody
Flags: firefox-backlog?

Updated

4 years ago
Flags: firefox-backlog? → firefox-backlog+

Updated

4 years ago
Flags: firefox-backlog+ → firefox-backlog?

Updated

4 years ago
Flags: firefox-backlog? → firefox-backlog+
You need to log in before you can comment on or make changes to this bug.