Closed Bug 278831 Opened 15 years ago Closed 13 years ago

Make Plugin Finder Service work with SeaMonkey

Categories

(Core :: Plug-ins, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: kairo, Assigned: twanno)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
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 ;-)
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 :)
Depends on: 279177
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.
Depends on: 270443
It should be possible to implement this without the info bar.
Clicking on the broken plugin would just invoke the wizard.
(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.
Is there a reason not to use the infobar?  Infobar has the advantage of being
accessible plus handling hidden plugins.
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.
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).
hopefully for the next version :)
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.
Do we still need work here in suiterunner?
I would think so; pluginfinderservice requires some additions to the frontend code.
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.
Depends on: suiterunner
suiterunner has landed, can we get this work updated and pushed into trunk?
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.
Depends on: 393857
Depends on: 400764
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.
Assignee: bugzilla → twanno
Status: NEW → ASSIGNED
Attachment #289013 - Flags: superreview?(neil)
Attachment #289013 - Flags: review?(neil)
We should use toolkit's contentAreaUtils where reasonably possible and only use our own for stuff that isn't present there.
Depends on: 404066
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)
Attachment #289013 - Attachment is obsolete: true
Attachment #289013 - Flags: superreview?(neil)
Attachment #289013 - Flags: review?(neil)
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...
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.
Attachment #289492 - Flags: superreview?(neil)
Attachment #289492 - Flags: review?(neil)
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]
Attachment #289492 - Flags: superreview?(neil)
Attachment #289492 - Flags: superreview-
Attachment #289492 - Flags: review?(neil)
Attachment #289492 - Flags: review+
Attachment #289492 - Attachment is obsolete: true
Attachment #289850 - Flags: superreview?(neil)
Attachment #289850 - Flags: review+
Attachment #289850 - Flags: superreview?(neil) → superreview+
Comment on attachment 289850 [details] [diff] [review]
make plugin finder service work for SeaMonkey (v3)

checked in by request of Teune via IRC.
This is completely fixed now that the fix for bug 400764 has been checked in.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.