225958 - file types/helper apps and plugins lists are empty; helper apps fixed

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Steffen Wilberg]

The "file types" section of Tools-Options-Downloads is empty, and so is the
plugins window.

WORKS:  2003111314
BROKEN: 2003111404

This is a regression from bug 223990, which renamed GetFromTypeAndExtension to
getFromTypeAndExtension.

Comment 1

[Steffen Wilberg]

Attachment: rename GetFromTypeAndExtension to getFromTypeAndExtension

This restores the file types/helper apps list. But the plugins window is still
empty. When opening the plugins window, the js console shows this error:

Error: uncaught exception: [Exception... "Component returned failure code:
0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIMIMEInfo.primaryExtension]" 
nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame ::
chrome://browser/content/pref/plugins.js :: anonymous :: line 136"  data: no]

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm...  I see nothing in the patch for bug 223990 which would have changed the
behavior of primaryExtension....  If you catch that exception and look at the
type of the MIMEInfo, what do you see?  This exception could be a dup of bug
225972...

Note, however, that primaryExtension _will_ throw for perfectly valid MIMEInfos,
so this code really does need to deal with that in the end.

Comment 3

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

>Hmm...  I see nothing in the patch for bug 223990 which would have changed the
>behavior of primaryExtension....

guess: due to bug 78919, a mimeinfo is now returned where previously an
exception would have been raised. the code fails to deal. bad luck for it.

Comment 4

[Steffen Wilberg]

I just found out that the timeframe is different. Sorry for not recognising earlier.
WORKS:  2003111506
BROKEN: 2003111616 (times converted to PST).
So it's this checkin for bug 78919:
http://bonsai.mozilla.org/cvsquery.cgi?date=explicit&mindate=11%2F15%2F2003+07%3A01&maxdate=11%2F15%2F2003+07%3A01

I catched the exception and added a couple of dump()s here:
http://lxr.mozilla.org/mozilla/source/browser/components/prefwindow/content/plugins.js#165
    var mimeInfo = this.getMIMEInfoForType(aMIMEType);
    dump ("mimeInfo: "+mimeInfo+"\n");
    try {
      var primExt = mimeInfo.primaryExtension;
    } catch (e) { dump("Exception with primaryExtension.\n"+e+"\n"); }
    dump ("primaryExtension: "+primExt+"\n");

getMIMEInfoForType() is found here:
http://lxr.mozilla.org/mozilla/source/browser/components/prefwindow/content/plugins.js#231

With 2003111506, the console shows this:
mimeInfo: [xpconnect wrapped nsIMIMEInfo]
primaryExtension: pict
mimeInfo: [xpconnect wrapped nsIMIMEInfo]
primaryExtension: wmv
mimeInfo: [xpconnect wrapped nsIMIMEInfo]
primaryExtension: asx
...

However, since 2003111616 (and using getFromTypeAndExtension instead of
GetFrom...), the output is this:
mimeInfo: [xpconnect wrapped nsIMIMEInfo]
Exception with primaryExtension.
[Exception... "Component returned failure code: 0xc1f30001
(NS_ERROR_NOT_INITIALIZED) [nsIMIMEInfo.primaryExtension]"  nsresult:
"0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame ::
chrome://browser/content/pref/plugins.js :: anonymous :: line 138"  data: no]
primaryExtension: undefined
mimeInfo: [xpconnect wrapped nsIMIMEInfo]
Exception with primaryExtension.
[Exception... "Component returned failure code: 0xc1f30001
(NS_ERROR_NOT_INITIALIZED) [nsIMIMEInfo.primaryExtension]"  nsresult:
"0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame ::
chrome://browser/content/pref/plugins.js :: anonymous :: line 138"  data: no]
primaryExtension: undefined

The js console shows another exception because I didn't catch other occurences
of primaryExtension.

So what's going on here?

Comment 5

[Boris Zbarsky [:bzbarsky]]

To repeat comment 2, "If you catch that exception and look at the type of the
MIMEInfo, what do you see?"

Also, just do a minimal run that shows the problem with NSPR_LOG_MODULES set to
HelperAppService:5 and NSPR_LOG_FILE set to some file, then attach the file to
this bug.

Comment 6

[Steffen Wilberg]

Attachment: console output

I've managed to get the plugins being displayed in its window by catching all
exceptions from primaryExtension. It turns out that primaryExtension does work,
but throw a lot of exceptions in between.

What do you mean by "type of the MIMEInfo"? The MIMEtype? I've attached the
console output from this code:

    try {
      var primExt = mimeInfo.primaryExtension;
      var type= mimeInfo.MIMEType;
      var desc = mimeInfo.Description;
    } catch (e) { }
    dump (primExt+" "+type+" "+desc+"\n");

Comment 7

[Steffen Wilberg]

Attachment: logfile

This is the logfile created with NSPR_LOG_MODULES = HelperAppService:5.

Although the plugins are being displayed now, the second last line in the
window shows empty extension and filetype columns. The last line shows a plugin
that should've been sorted in the list above.

Comment 8

[Boris Zbarsky [:bzbarsky]]

    try {
      var primExt = mimeInfo.primaryExtension;
      var type= mimeInfo.MIMEType;
      var desc = mimeInfo.Description;
    } catch (e) { }
    dump (primExt+" "+type+" "+desc+"\n");

Not useful.  You know the rimaryExtension getter throws, so the other getters
will never be called. Put that one last and rerun, please.  Better yet, put each
one in a separate try/catch.

As far as I can see, what's happening is that we have started returning a
MIMEInfo object even if the only bit of information we have is the MIME type. 
We didn't use to do that, but now we do.  Which means that when you do a lookup
on a type like "application/x-java-bean;version=1.2.2" (which is not necessarily
a valid string to pass to this function, by the way -- we should decide whether
it is, biesi) you get back a MIMEInfo with no default extension whereas before
you would have just gotten an exception thrown.

Comment 9

[Steffen Wilberg]

Attachment: console output v.2

I see. This is the console output from this code:
      try {
	var primExt = mimeInfo.primaryExtension;
      } catch (e) { }
      dump (primExt+" ");
      
      try {
	var type= mimeInfo.MIMEType;
      } catch (e) { }
      dump (type+" ");
      
      try {
	var desc = mimeInfo.Description;
      } catch (e) { }
      dump (desc+"\n");

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yeah, exactly.

Perhaps we should just change the primaryExtension getter to not throw.... and
check all its callers first.  biesi?  Thoughts?

Comment 11

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

>We didn't use to do that, but now we do.  Which means that when you do a lookup
>on a type like "application/x-java-bean;version=1.2.2" (which is not 
>necessarily a valid string to pass to this function, by the way -- we should 
>decide whether it is, biesi)

hm, I think that could be useful. maybe you want a different helper app for the
different version fields.

>Perhaps we should just change the primaryExtension getter to not throw.... and
>check all its callers first.  biesi?  Thoughts?

hm. I see no value in that. maybe the actual error code should be changed
(because the mi is initialized, it just has no extension - e.g. use
NOT_AVAILABLE), but throwing when no extension is available seems fine to me.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> hm, I think that could be useful. maybe you want a different helper app for
> the different version fields.

Then we need to fix the DoContent code to pass the actual type off the channel,
which would include that info....

In any case, this bug needs to be resolved on the client (Firebird) side, by
catching those exceptions.

I should note, without having read this code too carefully, that the fact that
this simply failed to display MIME types for which there is no MIME info was a
pretty bad bug....

Comment 13

[Steffen Wilberg]

I assume the backend won't change again in any way significant to this code.
So let me summarize:

Bug 223990 renamed GetFromTypeAndExtension to getFromTypeAndExtension.

Bug 78919 (part 2) started returning a MIMEInfo object even if the only bit of
information we have is the MIME type. The previous behaviour was to throw an
exception. primaryExtension throws an exception if there's no extension; I guess
that hasn't changed.

Patch upcoming.

Comment 14

[Steffen Wilberg]

Attachment: patch v.1

This patch renames the abovementioned function, catches the exceptions of
primaryExtension and only preceeds with addType if there is an extension.

Instead of "if (primExt)", I could've put the rest of addType in the "try"
part, but I think it's cleaner this way.
The "if (!(primExt in this._pluginTypeHash))... else..." part was just
indented, the only change is using primExt instead of
mimeInfo.primaryExtension, likes to throw exceptions.

The plugins window works just like it did before!

Comment 15

[Steffen Wilberg]

Comment on attachment 135911 [details] [diff] [review]
patch v.1

Ben should probably review this, but Bugzilla says he's away sick at the
moment.
Pierre, do you want to jump in?

Comment 16

[Steffen Wilberg]

-> All/all. Taking.

Comment 17

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

>I assume the backend won't change again in any way significant to this code.

"probably". but I will not promise that. the interface is not frozen.

Comment 18

[Steffen Wilberg]

I'd just like this patch to be in before something breaks it. :-)
On the other hand, my patch doesn't depend on anyone throwing or not.

Comment 19

[Boris Zbarsky [:bzbarsky]]

> I assume the backend won't change again in any way significant to this code.

I assume it will; just not in this milestone cycle.  ;)  And if it does, it'll
change in ways that should hopefully make us update this code in sync...

> and only preceeds with addType if there is an extension.

Doesn't that simply reintroduce the original bug?  The one that kept many of the
plugin-related types from appearing in the list at all?  I guess that's ok as a
stopgap measure, but someone should fix that at some point....

In any case, the patch you attached is about the cookieviewer, not this bug.

Comment 20

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

>I'd just like this patch to be in before something breaks it. :-)

ah, that should be no problem :) the tree is frozen now.

Comment 21

[Steffen Wilberg]

Comment on attachment 135911 [details] [diff] [review]
patch v.1

wtf! wrong patch. But Pierre, I don't mind if you review it, it's bug 75119.
;-)

Comment 22

[Steffen Wilberg]

Attachment: patch v.1, the real one

bz: I filed this regression bug because there were no plugins at all in the
plugins window. So my primary goal was to restore that list.

You're right, this code didn't display plugins without an extension, and my
patch doesn't change that. But I'm not convinced that we need to change this. I
don't want to have 28 or so java entries in the window; we've got a single
checkbox for java already.

Comment 23

[Steffen Wilberg]

Comment on attachment 135933 [details] [diff] [review]
patch v.1, the real one

This is the right patch. Pierre, r?

Comment 24

[Boris Zbarsky [:bzbarsky]]

steffen, it'll be a problem for more than Java... This code should be getting
the extension list from the plugin, not from the MIME service, basically.

Comment 25

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

yeah well. that code should also not disable plugins by removing
gecko-content-viewer category entries (because that will not work for in-page
plugins)

Comment 26

[Steffen Wilberg]

bz: You mean like about:plugins?  What's the difference/advantage of doing that?
I've got all my plugins listed in that window with their primary extension,
except Java. I don't miss anything I always wanted to disable.

biesi: That's right, you can't disable e.g. embedded flash animations by
disabling the plugin. But the current concept of the plugins window is to give
the user the choice between displaying or downloading. It's not about hiding
content. Its description reads: "Disabling Plug-Ins for a file type will cause
files of that type to be downloaded instead of being viewed in the Plug-In."

Therefore the plugins button is located in the Downloads section of Firebird's
options. A content blocker (based on MIME type) would have to be located in the
Web Features section, next to the popup and image manager. But I don't think we
need  anything like that. The only content I'd like to hide or stop are animated
images ( bug 213377 ) and flash animations (already available via All-in-One
Gestures/Mouse Gestures/Flash-Click-to-View extensions).

Is using Gecko-Content-Viewers a bad idea apart from that?

Comment 27

[Boris Zbarsky [:bzbarsky]]

> I've got all my plugins listed in that window with their primary extension,

Because they happen to map to types your OS knows about and has extensions for.
 That's not guaranteed by any means, especially if you install some non-standard
plugin...

> Therefore the plugins button is located in the Downloads section of Firebird's
> options.

It's probably not staying that way per my last conversation with Ben, btw...  At
that point this whole thing will be moot anyway.

Comment 28

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

>Is using Gecko-Content-Viewers a bad idea apart from that?

?
gecko-content-viewers is what the code _already_ uses, in order to implement the
view/download choice.

Comment 29

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

sorry I misunderstood you
well... doing it that way has the disadvantage that on each startup, the
category entry has to be removed, and you have to hope that it doesn't get
restored (eg does about:plugins restore it?)

Comment 30

[Steffen Wilberg]

bz: I don't install non-standard plugins I'd like to disable after that...

> It's probably not staying that way per my last conversation with Ben, btw...
> At that point this whole thing will be moot anyway.
Yeah, Ben plans something like "Download System Upgrade (ongoing Helper
Applications and Download Manager updates), Part II".
Hey, I love innovation! That is, as long as my patches don't bitrot.

biesi: Opening about:plugins doesn't enable plugins I disabled beforehand,
although it says the plugin is enabled.

Comment 31

[Ali Ebrahim]

*** Bug 226382 has been marked as a duplicate of this bug. ***

Comment 32

[Ali Ebrahim]

*** Bug 224466 has been marked as a duplicate of this bug. ***

Comment 33

[Steffen Wilberg]

Scott fixed helperApps.js on 11/23/2003 13:28.
My last patch is still needed for the plugins.

Comment 34

[Justin Kremer]

This issue appears to be occuring only with the Installer version.  Tested using
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031124
Firebird/0.7+

Comment 35

[Ali Ebrahim]

My plugins list is empty, and I'm not using the installer version. Also the fact
that its All/All suggests that its not installer-related.

Comment 36

[Steffen Wilberg]

Comment on attachment 135933 [details] [diff] [review]
patch v.1, the real one

Ben, Bugzilla no longer says you're "away sick". Glad to hear that. Please have
a look at this.
Explanations in comment 13 and comment 14.

Comment 37

[Ben Goodger (use ben at mozilla dot org for email)]

Boris is right, the UI here will change somewhat in the coming weeks. Since the
Plugins options here really control how file types are handled when links are
opened, they are more applicable to _downloads_ than they are to in-page
plugins. That was the intent. I am going to merge the list in the Plugins dialog
with the file type handler list hopefully before 0.8.

Comment 38

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 135933 [details] [diff] [review]
patch v.1, the real one

r=[e-mail]

... but until I do change the behaviour feel free to check this in and repair
the current functionality for now.

Comment 39

[Steffen Wilberg]

Comment on attachment 135933 [details] [diff] [review]
patch v.1, the real one

Can anybody check this in, please? I don't have a cvs account.

Comment 40

[Hussam Al-Tayeb]

plugins list are empty in 20031201

Comment 41

[Boris Zbarsky [:bzbarsky]]

I assume checkins to mozilla/browser do not require driver approval at the moment?

Comment 42

[Mike Connor [:mconnor]]

bz: that's correct, so if you would be so inclined it'd be nice

Comment 43

[Ben Goodger (use ben at mozilla dot org for email)]

Patch checked in. Thanks Steffen!

Comment 44

[Jesse Ruderman]

The plugins list is still empty for me in
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031207
Firebird/0.7+

Comment 45

[Steffen Wilberg]

wfm.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031207
Firebird/0.7+ (Steffen)

Did you try a new profile?

Comment 46

[Simon Paquet [:sipaq]]

verified fixed 2004-01-31 trunk build on W2K

Comment 47

[f4lc0n]

If I have java disabled, and do not have firefox select a profile using the
profile manager, ie start with parameters -p f4lc0n, the plugin window is empty.

Comment 48

[f4lc0n]

Im using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040328
Firefox/0.8.0+ but it has been happening for a while sorry for the additional
post  I didnt see an edit option anywhere.

Comment 49

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change