directory listings shouldn't rely on libappcomps

VERIFIED FIXED in mozilla0.9.9

Status

()

Core
Embedding: APIs
P1
normal
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: amutch, Assigned: bbaetz)

Tracking

Trunk
mozilla0.9.9
x86
Windows 2000
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

17 years ago
From Bugzilla Helper:

BuildID:    MFCEmbed - 11/19/2001 nightly

When browsing to any FTP site with MFCEmbed, instead of listing the site 
directory listing, a dialog box titled "Choose An Action" appears and indicates 
that "You have chosen to download a file of type: applicaton/http-index-
format". You have the option of "Open Using" and you can select and application 
or "Save this file to a disk".  Neither option will properly open the FTP 
site.  

Reproducible: Always

Expected Results:  MFCEmbed should list the contents of the FTP directory of 
the site that you have opened.
(Reporter)

Updated

17 years ago
Summary: MFCEmbed: No FTP support → MFCEmbed: No FTP directory listing
(Assignee)

Comment 1

17 years ago
Do you include  the appcomps lib?
(Reporter)

Comment 2

17 years ago
OK, if I add in the appcomps.dll from the latest Mozilla nightly, the FTP 
directory site listing works. However, appcomps.dll is not normally included 
with embedded versions of Mozilla.  Are we going to have to include this file in 
our distributions to properly enable FTP directory listing? Should that be part 
of the "core" files for the embedded version of Mozilla?
(Assignee)

Comment 3

17 years ago
Not as such. The problem is that it can't go into necko, because it involves a
layout interface. I need to slightly refactor some of the code, I think, for
this and a couple of other bugs.
Assignee: adamlock → bbaetz
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P4
Summary: MFCEmbed: No FTP directory listing → directory listings shouldn't rely on libappcomps
Target Milestone: --- → mozilla0.9.7

Comment 4

17 years ago
Bradley are you taking this?

To explicitly state the problem, it appears that the directory viewer service
(the thing that displays FTP sites as HTML) lives in
mozilla/xpfe/components/directory and thus gets built into appcomps.dll.
Embedding doesn't want to include this file because it's a monster.
An alternative location might be mozilla/embedding/components but it might be
worth floating the idea on npm.embedding first.

Comment 5

17 years ago
Sorry you already own it so ignore that bit :)
(Assignee)

Comment 6

17 years ago
My plan is to have necko get the pref status, and do the html conversion there
if we need to.

This means that embedders will have to set the pref to use the html view to on
explicitly if they want it to work.
Status: NEW → ASSIGNED
(Assignee)

Updated

17 years ago
Blocks: 102826
(Assignee)

Comment 7

17 years ago
This has missed 0.9.7, and so moving to 0.9.9 (I'm not here for 0.9.8). Bug
102812, bug 102826, and bug 110760 will probably be fixed by the same fix, and
so nominating for 1.0, since I expect that embedders will want bug 110760 fixed,
at least.
Keywords: mozilla1.0
Priority: P4 → P1
Target Milestone: mozilla0.9.7 → mozilla0.9.9
(Assignee)

Comment 8

17 years ago
*** Bug 121473 has been marked as a duplicate of this bug. ***

Updated

17 years ago
QA Contact: mdunn → depstein

Comment 9

17 years ago
cc'ing alec for more dependency thoughts.

Comment 10

17 years ago
from the FAQ (http://www.mozilla.org/projects/embedding/faq.html#4)
-----
Why won't FTP work?

Make sure you are including the "necko2" DLL and XPT files. If you are able to
download ftp files, but cannot show directories, you may be using the wrong FTP
directory view. There are two different views to display FTP directories. One of
these views is XUL-based and requires the "appcomps" DLL. The other view is
HTML-based and requires only the necko2 DLL. The network.dir.generate_html
preference will make mozilla use HTML mode.

Note: there have been reports that appcomps.dll is still needed, even with HTML
displays... this is a bug that needs to be investigated further. In fact, we
should fallback to HTML if the directory viewer isn't found anyway. -alecf
----

Now, if we need appcomps just to display the HTML listing (not the XUL-based
one) then something is busted here, and it's something that we should be able to
fix without adding extra dependencies, etc. My guess is that it's just some
bogus early-failure. It's just a matter of debugging to find out where the
failure is occuring and handle it gracefully.

(Assignee)

Comment 11

17 years ago
No, the "what type do we use" stuff relies on appcomps, and it can't be moved to
necko w/o making necko depend on appcomps. I know what I have to do, and this
bug is next on my list after I get rid of some regression bugs I have/
(Assignee)

Updated

17 years ago
Blocks: 123991
(Assignee)

Comment 12

17 years ago
I'm hacking on a patch now, which works for ftp. I'm going to attach it for
comments. Its not complete (it doesn't touch gopher or file yet), but it works
for ftp. Note that I'd love to move the pref stuff into a separate mixin helper
class, but since the protocols are in different shared libs, that would either
require putting the entire class in the (exported) header file or linking
libnecko2.so to libnecko.so explicitly, and I don't imagine that either of those
would be appropriate. Instead, I'm going to duplicate code in three places.
Ain't XPCOM fun.

Note that the actual display of application/http-index-format files sent via a
webserver will still depend on libappcomps. Once bug 123991 is fixed, then I can
probably move that into layout. No promises though, but since thats no different
to how its always been, and I've never seen a webserver which actually serves
those, so I'm not too worried.
Blocks: 102812, 115135
(Assignee)

Comment 13

17 years ago
Created attachment 69826 [details] [diff] [review]
v0.9 - for comment

Darin, dougt, valeski, anyone else? Comments?

One thing I didn't mention above - I decided to ddefault to html because thats
the generic format. If you want me to default to the pref, then I need to fix
all the callers, and I'm worried that that may be a large number, and would
impose an additional requirement everywhere - its easier to localise here in
the one place all documents go through.
(Assignee)

Comment 14

17 years ago
darin, dougt - please see my above comments + the patch. Its not ready for
review, but if you hate the design concept, please let me know ;)
(Assignee)

Updated

17 years ago
Depends on: 126417
(Assignee)

Comment 15

17 years ago
Created attachment 70400 [details] [diff] [review]
v1

Darin didn't have problems with the concept, so here we go. This doesn't change
file:///, becuase of the i18n issues - there's no point in adding the
infrastructure just to only support html.

I've included what is probably a patch to fix the i18n issues in here - after
this is in I'll get someone who can test to check, before making the file:///
changes for this interface in the other bug.

This includes a workarround for a streamconverter crash, and I changed the pref
stuff in the debug pref panel.

The old pref is unsupported - it was debug only anyway, so I don't see that as
an issue. If this goes in, I'll post to .netlib and .embedding describing the
change.

Also, caching the pref in the protocol handlers is not really a win, since they
aren't set up with a static ::get() method like http is, and the code to do
that + set up the observer is more than the code to check the pref each time.

I'd like to get this in today, if possible - can I get some review? I'll also
need the .idl added to the mac build. I'll find someone on irc to do that for
me.
Attachment #69826 - Attachment is obsolete: true

Comment 16

17 years ago
Comment on attachment 70400 [details] [diff] [review]
v1

sr=darin
Attachment #70400 - Flags: superreview+

Comment 17

17 years ago
Comment on attachment 70400 [details] [diff] [review]
v1

what darin said.

Also get a mac person to help you add your idl so that you get typelibs and
all.
Attachment #70400 - Flags: review+
(Assignee)

Comment 18

17 years ago
Checked in. ftp and gopher no longer requite libappcomps. File still does - see
bugs 102812 and 115135.
(Assignee)

Comment 19

17 years ago
And marking fixed, too
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 20

17 years ago
awesome!
we need to updating the embedding FAQ then
(Assignee)

Comment 21

17 years ago
Yeah, probably. I'll wait til I have file hacked up. For various reasons that
won't be befor ethe weekend, then someone has to test it, and then we'll see.
verified fixed on mfcembed 2002-02-20-09-trunk and testgtkembed 
2002-02-20-06-trunk
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.