Last Comment Bug 278831 - Make Plugin Finder Service work with SeaMonkey
: Make Plugin Finder Service work with SeaMonkey
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: ---
Assigned To: Teune van Steeg
:
Mentors:
Depends on: 279177 270443 suiterunner 393857 400764 404066
Blocks:
  Show dependency treegraph
 
Reported: 2005-01-18 05:36 PST by Robert Kaiser
Modified: 2007-12-20 07:37 PST (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
make plugin finder service work for SeaMonkey. (12.00 KB, patch)
2007-11-16 09:18 PST, Teune van Steeg
no flags Details | Diff | Splinter Review
make plugin finder service work for SeaMonkey (v2). (13.59 KB, patch)
2007-11-20 06:44 PST, Teune van Steeg
neil: review+
neil: superreview-
Details | Diff | Splinter Review
make plugin finder service work for SeaMonkey (v3) (14.02 KB, patch)
2007-11-22 13:46 PST, Teune van Steeg
twanno: review+
neil: superreview+
Details | Diff | Splinter Review

Description Robert Kaiser 2005-01-18 05:36:24 PST
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 2005-01-18 06:03:29 PST
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] 2005-01-18 06:53:53 PST
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] 2005-03-28 01:30:53 PST
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 neil@parkwaycc.co.uk 2005-03-28 15:57:44 PST
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] 2005-03-28 16:02:29 PST
(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) 2005-03-28 19:50:10 PST
Is there a reason not to use the infobar?  Infobar has the advantage of being
accessible plus handling hidden plugins.
Comment 7 timeless 2005-03-28 19:59:04 PST
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) 2005-03-28 21:15:54 PST
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 2005-03-29 12:29:45 PST
hopefully for the next version :)
Comment 10 neil@parkwaycc.co.uk 2005-04-01 15:24:14 PST
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 2007-05-20 07:26:27 PDT
Do we still need work here in suiterunner?
Comment 12 Christian :Biesinger (don't email me, ping me on IRC) 2007-05-20 07:56:20 PDT
I would think so; pluginfinderservice requires some additions to the frontend code.
Comment 13 Robert Kaiser 2007-05-20 08:55:23 PDT
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 2007-06-08 14:14:25 PDT
suiterunner has landed, can we get this work updated and pushed into trunk?
Comment 15 Teune van Steeg 2007-08-28 08:45:41 PDT
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 2007-11-16 09:18:25 PST
Created attachment 289013 [details] [diff] [review]
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 2007-11-16 09:48:28 PST
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 2007-11-16 16:45:09 PST
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 neil@parkwaycc.co.uk 2007-11-17 16:12:41 PST
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 2007-11-20 06:44:42 PST
Created attachment 289492 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2007-11-21 07:14:24 PST
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 2007-11-22 13:46:51 PST
Created attachment 289850 [details] [diff] [review]
make plugin finder service work for SeaMonkey (v3)
Comment 23 Robert Kaiser 2007-11-24 16:34:05 PST
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 2007-12-20 07:37:03 PST
This is completely fixed now that the fix for bug 400764 has been checked in.

Note You need to log in before you can comment on or make changes to this bug.