278831 - Make Plugin Finder Service work with SeaMonkey

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Robert Kaiser]

The old nullplugin dialogs that pop up on missing plugins in SesaMonkey aren't
very ideal. Now that we have a much better "Plugin Finder Service" that works
with Firefox, we should get this to work with SeaMonkey as well.

Comment 1

[Robert Kaiser]

BTW, I assigned that to Frank Wein (mcsmurf) because he has done some work on
that already (he hasn't attached a patch, as his approach still has some known
problems). I just want to let people know that there's actually work done here ;-)

Comment 2

[Frank Wein [:mcsmurf]]

The current work-in-progress patch can be found unter
http://www.mcsmurf.de/pfs_1.diff
Current problems are:
- How to get build id (solved partly, code just appends 00 for the hours to the
build date here)
- GUID, hardcoding the Mozilla GUID isn't so good here IMHO (Firefox uses
nsIXULAppInfo here to get the build id and the GUID)
- Plugin replacement text/icon doesn't appear: Go for example to
http://www.macromedia.com/shockwave/welcome/ and observe that it doesn't appear.
When you switch to another tab and back or trigger a mousedown event where the
plugin content should be (meaning: click on it), the replacement text/icon
appears (this already worked, probably i made a wrong change to the code).
Fixes or suggestions for any of these problems are welcome :)

Comment 3

[Frank Wein [:mcsmurf]]

So i did some testing, it seems the info-bar (like it is used in Firefox for
missing plugins, etc.) circumvents this reflow-not-happening problem, marking
dep. Given that one can't disable this info-bar (for now?), i think this is a
reasonable workaround.

Comment 4

[[e-mail]]

It should be possible to implement this without the info bar.
Clicking on the broken plugin would just invoke the wizard.

Comment 5

[Frank Wein [:mcsmurf]]

(In reply to comment #4)
> It should be possible to implement this without the info bar.
> Clicking on the broken plugin would just invoke the wizard.

Well, see the other bug this depends on, layout bug blocking this atm.

Comment 6

[Doron Rosenberg (IBM)]

Is there a reason not to use the infobar?  Infobar has the advantage of being
accessible plus handling hidden plugins.

Comment 7

[timeless]

and being annoying and not guaranteed to be available. my product is unlikely to
ever use an info bar. it isn't accessible for us.

Comment 8

[Doron Rosenberg (IBM)]

and your product actually cares about installing plugins? Note that in the
toolkit version, clicking on the missing plugin piece does invoke the wizard as
well.  We could pref the infobar (there has been the same request for the
toolkit version).

Comment 9

[timeless]

hopefully for the next version :)

Comment 10

[[e-mail]]

Actually on second thoughts I'd quite like there to be a context menu item to
get the plugin - clicking involves setting up all those event handlers, but we
already have a fine context menu system.

Comment 11

[Robert Kaiser]

Do we still need work here in suiterunner?

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

I would think so; pluginfinderservice requires some additions to the frontend code.

Comment 13

[Robert Kaiser]

OK, from a glance at the WIP patch (that is still there), I see that it needs hooks in SeaMonkey-specific stuff - it'll probably be a lot easier with suiterunner, so marking suiterunner as a dependency.

Comment 14

[Robert Kaiser]

suiterunner has landed, can we get this work updated and pushed into trunk?

Comment 15

[Teune van Steeg]

I'd like to take this bug.
I'm planning to get the basics of this bug done in bug 393857. (code that can be shared in toolkit)
The SeaMonkey specific code will be handled in this bug.

Comment 16

[Teune van Steeg]

Attachment: make plugin finder service work for SeaMonkey.

Two notes:
1) This requires the patch from bug 400764.
2) The following code assumes toolkits makeURI is available:
> makeURI(pluginsPage, doc.characterSet, doc.documentURIObject).spec
but in this case only suites makeURI is available (suites makeURI only expects two arguments).
Is there a plan to move from suites contentAreaUtils to toolkits contentAreaUtils? Otherwise the third argument can just as well be left out.

Comment 17

[Robert Kaiser]

We should use toolkit's contentAreaUtils where reasonably possible and only use our own for stuff that isn't present there.

Comment 18

[Teune van Steeg]

Comment on attachment 289013 [details] [diff] [review]
make plugin finder service work for SeaMonkey.

first, there are some other changes to be done (bug 404066)

Comment 19

[[e-mail]]

Comment on attachment 289013 [details] [diff] [review]
make plugin finder service work for SeaMonkey.

>+            var tagMimetype;
Why not name this just mimetype?

>+              if (pluginElement instanceof HTMLObjectElement) {
>+                pluginsPage = pluginElement.getAttribute("codebase");
You can use pluginElement.codebase here which is already an absolute URI. (It's unfortunate that there's no equivalent for embed elements.)

>+                  pluginsPage = makeURI(pluginsPage, doc.characterSet, doc.documentURIObject).spec;
You're probably better off writing this out in full - XBL bindings shouldn't depend on global scripts.

>+              if (tagMimetype == "") {
if (!mimetype)

>+            var eventHandler = {
>+              notificationbox: this,
>+              handleEvent: function(aEvent) {
<implementation implements="... nsIDOMEventHandler"> perhaps?

>+                window.openDialog("chrome://mozapps/content/plugins/pluginInstallerWizard.xul",
>+                                  "PFSWindow", "chrome,resizable=yes",
Do you really want to reuse an existing PFSWindow if one is already open?

>+          try {
>+            if (gPrefService.getBoolPref("plugins.hide_infobar_for_missing_plugin"))
>+              return;
>+          } catch (ex) {}
This shouldn't need try/catch.

>+          var browser = this.activeBrowser;
>+          if (!browser.missingPlugins)
>+            browser.missingPlugins = {};
Is that necessary, or can you use a field?

>+missingpluginsMessage.button.label=Install Missing Plugins...
>+missingpluginsMessage.button.accesskey=I
>+
> xpinstallHostNotAvailable=unknown host
> xpinstallPromptWarning=%S prevented this site (%S) from asking you to install software on your computer.
> xpinstallPromptInstallButton=Install...
Might be worth renaming this to Install Software...

Comment 20

[Teune van Steeg]

Attachment: make plugin finder service work for SeaMonkey (v2).

Patch updated as per comment 19

> XBL bindings shouldn't depend on global scripts
This binding relied on the globally defined variable 'pref', I changed that to make this binding have it's own field for the preference service.

Comment 21

[[e-mail]]

Comment on attachment 289492 [details] [diff] [review]
make plugin finder service work for SeaMonkey (v2).

>+      <field name="_prefs" readonly="true">
>+        <![CDATA[
>+          Components.classes['@mozilla.org/preferences-service;1']
>+                    .getService(Components.interfaces.nsIPrefService);
>+        ]]>
>+      </field>
This should be nsIPrefBranch2; you're accidentally making it work because the constructor has its own QI (which you won't then need any more).

>+                    var prefService = this._prefs;
Should be prefBranch (or prefs if you prefer).

>+      <field name="missingPlugins">null</field>
Should be {} so you don't have to check again later.
[Aren't you supposed to reset this when you navigate to a new page?]

>+          this.missingPlugins = null;
[Should be {} again]

Comment 22

[Teune van Steeg]

Attachment: make plugin finder service work for SeaMonkey (v3)

Comment 23

[Robert Kaiser]

Comment on attachment 289850 [details] [diff] [review]
make plugin finder service work for SeaMonkey (v3)

checked in by request of Teune via IRC.

Comment 24

[Teune van Steeg]

This is completely fixed now that the fix for bug 400764 has been checked in.