Closed Bug 52454 Opened 24 years ago Closed 24 years ago

Fix Downloading... dialog and general helper app stuff

Categories

(SeaMonkey :: UI Design, defect, P1)

Product:
SeaMonkey
Component:
UI Design
Platform:
x86
Linux
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.1

People

(Reporter: ewv, Assigned: law)

References

Details

Attachments

(9 files)

Patches to uriloader and helper app prefs to work with new helper app dialog
12.04 KB, patch
Details | Diff | Splinter Review
Updated patch with lots of incidental bugs fixed
24.15 KB, patch
Details | Diff | Splinter Review
Patch to build system stuff to excise old ucth and build new helper app dialog
9.30 KB, patch
Details | Diff | Splinter Review
patch to remove #include of removed .h file
1.15 KB, patch
Details | Diff | Splinter Review
Updated patch to uriloader and prefs helper apps dialogs (with a couple of minor fixes per comments from Paul and a bug found during Linux testing)
26.50 KB, patch
Details | Diff | Splinter Review
Detailed explanation of changes to uriloader stuff
10.32 KB, text/plain
Details
Additional Win32 patch to get better description and set "alwaysAsk" flag from registry
4.31 KB, patch
Details | Diff | Splinter Review
New Win32 patch (fixes case where horked registry contents cause leak)
4.37 KB, patch
Details | Diff | Splinter Review
Patch to handle .xul file with MIME type "text/xul"
4.47 KB, patch
Details | Diff | Splinter Review 
Adding an application in the downloading window and unchecking the "always ask"
box should put that application for that mime type in the helper applications
list. As it is, unchecking the box has no effect. The box is checked again by
default and mozilla has no idea what to do with the mime-type the next time it
is clicked on.


Build: 2000090808
shrir/mscott, is this you...?
QA Contact: sairuh → shrir
We didn't implement this feature for seamonkey so the functionality got featured.

I reckon i could disable the check box or something to avoid confusion?
mscott/shrir, d'you have a bug number for that futured feature? could you pls cc
me and eric on it? thx!

eric, what d'you think about mscott's suggestion? methinks that'd do for me,
since this feature is on hold till after 6.0 anyhow...
Status: UNCONFIRMED → NEW
Ever confirmed: true
That would definitely lessen confusion. Thanx.
looks like the checkbox was mysteriously disable recently... :)
nav triage team: this is a beta stopper.
Keywords: nsbeta1
Priority: P3 → P1
bill shd take this.
Assignee: ben → law
Setting target milestone and changing summary.
Summary: Choosing app or selecting "always ask" in "Downloading" window doesn't remember → Enable "always ask me" option on Downloading... dialog
Target Milestone: --- → mozilla0.8
Here's the deal...

To fix this, it appears we must make some substantive changes to both the
"Downloading..." dialog (and related front-end code), and, to the underlying
"back end" code in mozilla/uriloader/exthandler.

The biggest glitch is that currently the only thing we store in our RDF mimetype
datasource (i.e., mimeTypes.rdf file) are the limited set of helper applications
that the users adds via the Navigator/HelperApplications prefs panel.  However,
the Downloading... dialog also appears for mime types that are mapped to helper
applications via platform-specific means (e.g., the registry on Windows,
Internet Config on Mac).  This means we have no place to store the "don't always
ask me" flag.

So, we must add entries in our data source for at least some of the
system-defined mime-type-to-helper-app mappings.  The tricky thing is that we
don't necessarily want to store the mime-type to helper-app mapping there.  The
reason is that this mapping is really defined by the system and one would
probably want changes in the system setting to still apply.

Conceptually, that's not too hard.  We just need to record that the mapping for
some entries in our data source are "default" and that we should continue to
rely on the system mapping (independent of the "don't always ask me" flag).

In practice, that's where it gets complicated.  The back-end code works like
this (simplified greatly):
1. Call platform dependent implementation of DoContent method to try to find a
helper app for the mime type.
2. Platform-dependent function calls common XP base class implementation of
DoContent to look for entry in our data source.
3. If 2 fails, try to find system-defined mapping in a platform dependent manner.
4. If that fails, then use a "null" helper app (that requires the user to
specify a helper app before the Downloading... dialog will permit the "open
using" option).

To properly handle the "don't always ask me" setting, the logic will have to be
something like:
1. As above.
2. As above, except the common xp method has to return one of 3 states (rather
than simply pass/fail): pass (user-defined helper app mapping found), fail
(nothing found for this mime type), or "partial success" (entry in data source
doesn't tell us the helper app, just the "don't always ask me" flag).
3. If 2 either fails or is a partial success, get system-defined mapping.  In
case of a partial success at step 2, overlay "don't always ask me" setting (the
default is "always ask me").
4. Same as above.

The other issues are mostly front-end issues, but may impact the backend,
depending on how we choose to implement them.  Currently, the front end is
isolated from the implementation details of the RDF helper app data source, so I
think we should try to put the RDF code that updates that datasource in the back
end.

Anyway, the front end issues are:

a. I think the user needs to explicitly ask that the settings for a given mime
type be saved.  If we automatically presume that the choice they make for a
given instance of a given content should be applied to future occurrences of
that content, then I think the user might get confused.  For example, I might
choose to open a specific .doc file in notepad because I know it's not really of
type application/ms-word.  So I get to the Downloading... dialog, choose
notepad, and say "open using."  If the next time I load a .doc file and it uses
notepad again, that probably won't be desired (or expected) behavior.  So I
think it would be better to give the user a way to say "this really is the
helper app I always want to use for this mime type".  My idea is to add a
"Always use this helper app when opening files of this type" checkbox under the
Open Using portion of the Downloading... dialog.  This would be checked and
disabled on initial display of the dialog.  If the user uses the Choose...
button to select a new helper app, then the checkbox is enabled and unchecked.

b. The other issue relates to the use of the Downloading... dialog when we
encounter an entirely new content-type for which we have no helper app (step 4
in the logic described above).  In that case, if the user selects a helper app,
we should permit the user to indicate that this selection should be used next
time they encounter content of this type.  That can be accomplished by using the
same checkbox described in a., above.

c. Another issue is whether (and how) to permit the user to save their selection
of "open using" vs. "save to disk."  First, this setting is another (like the
"always ask me" setting) that must be saved for both pref-defined and
system-defined mime-type-to-helper-app mappings.  But that problem would be
solved the same way as for the "always ask me" attribute.  One issue is whether
the open vs. save choice should be automatically remembered.  I suspect not, for
much the same reason that the choice of helper app ought not be automatically
remembered.  But if not automatic, then the user needs yet another checkbox to
tell us to remember this choice.  Now the dialog might be getting too complex.
But I'm going to implement it that way until a better suggestion is provided.

d. The last front-end issue is that the full set of attributes for a given
mime-type-to-helper-app mapping need to be exposed on the prefs panel.  That
means we need to reveal the "always ask me" setting (especially since that's the
only means by which it might be turned back on!) and the "open vs. save"
setting.  The pref panel (and supporting dialogs) would need to add support for
this.  In addition, that code will also need to deal with the separate class of
entries that will be used to attach the extra attributes to system-defined mappings.
Status: NEW → ASSIGNED
*** Bug 57810 has been marked as a duplicate of this bug. ***
Moving out to mozilla0.9.
Target Milestone: mozilla0.8 → mozilla0.9
*** Bug 71415 has been marked as a duplicate of this bug. ***
nav triage team:

Adding nsbeta1+
Keywords: nsbeta1 → nsbeta1+
I've checked in a new "helper app dialog" component 
into /mozilla/embedding/components/ui.  There some stuff missing for Linux/Mac 
still.

This replaces the "unknown content type component" in xpfe/components/ucth.

To use this (for now):

1. Delete components/*ucth*
2. go to embedding/components/ui and do an "nmake -f makefile.win"
3. Apply the patch attached to this bug.
4. Rebuild.

There's some stuff missing still (particularly, the "always ask me" checkbox is 
still disabled).

There are a number of flaky problems that I have to figure out, also.  I'm 
providing this patch to enlist help with some of the RDF issues.
*** Bug 73590 has been marked as a duplicate of this bug. ***
Blocks: 75093
Blocks: 70228
Blocks: 49787
OK, things mostly work with the latest patch I just attached (along with the 
new helper app dialog code in embedding/components/ui/helperAppDlg).

What remains are:

1. Go to windows registry to decide on the "always ask me" setting for mime 
infos generated from the Windows version of the nsOSHelperAppService.cpp code.  
I just haven't got to that detail yet.

2. Enable the uriloader and prefs helper app edit dialog to deal 
with "useSystemDefault."  I plan to use that "preferred action" to deal with 
user-overrides (e.g., for "always ask me") on top of mime types whose handling 
are otherwise based on OS settings.

3. Some build-system tweakage to remove the old xpfe/components/ucth stuff and 
build embedding/components/ui/helperAppDlg.

Re: item 2: I've removed the "always ask me" checkbox from the helper app 
dialog.  The only way to turn this off is to press the "Set Default..." button 
and do it there.  This will have the side-effect of creating user-specified 
helper apps for the mime type.  The problem with this is that if you change the 
OS settings for that extension/mimetype, then the info we store in 
mimeTypes.rdf will override that setting.


I added a patch for the various make system files.
Applied patch to my powerbook, building right now. There is one slight wrinkle
that I know of, I put a string inside one of the properties files inside ucth,
something like helperAppLauncher.properties. Guess I gotta find a new home for
it.

Just attached patch that fixes some mac bustage. Also, need to fiddle with 
mozcompsIDL.mcp to remove obsolete interfaces, add nsIHelperAppLauncherDialog.idl 
to EmbedComponentsIDL.mcp, also need to add MANIFEST file to mozilla/embedding/
components/ui/helperAppDlg (I think Bill just forgot it)

Right now, the download progress window is now one of those full screen blank 
windows. Also, I set the download to launch StuffIt Expander and that also didn't 
happen.

Thanks, Paul.  This is not going in today, BTW.

re: #includes of nsIUnkContentTypeHandler.h

Yes, there may be some stray instances kicking around.  I'll check for more of
those and clean them up.

re: new MANIFEST file

Yes, I have it.  It just doesn't show up in my patch.

re: StuffIt Expander not working

Does the old helper app dialog launch it?  I seem to recall a comment in the
code to the effect that the Mac's "spawn" didn't support command line arguments,
or something.  I've been fixing most underlying bugs in the
nsExternalHandlerAppService as I find them but some we won't see till the code
gets tested on all three platforms.
Spam: new helper app dialog not making mozilla0.9, unfortunately.
Target Milestone: mozilla0.9 → mozilla0.9.1
OK, moving back to mozilla0.9 'cause these are "embedding" bugs.
Target Milestone: mozilla0.9.1 → mozilla1.2
Adding ETA to status whiteboard.  Remaining tasks are to test out more 
thoroughly on Mac/Linux and get appropriate reviews.
Whiteboard: Apr 19
Target Milestone: mozilla1.2 → mozilla0.9
Blocks: 73102
sr=ben for the changes to the helper app dialog & preferences. 

Re-applied patches to a new tree yesterday on my powerbook. Looks good. There's 
a seperate issue where the prefs dialog is getting a horked path so the helper 
app isn't getting launched. That's when you click "Set Default" in the new 
dialog. I think it's a pref dialog issue because if you don't click the "Set 
Default" or use the override on the bottom of the dialog, it works fine.

r=pchen
Bill, you can move this to 0.9.1, unless you have other need for it in 0.9.
Um, didn't *you* want this for mozilla0.9?  It has the fixes for 70228 and 
73102, among others.

Code is done.  I'm just awaiting review/approval/super-review of the uriloader 
changes from mscott.

Does anybody else care to review that code?  I've just attached a very detailed 
explanation of the changes; hopefully that will make the reviewing trivial.
Whiteboard: Apr 19 → Apr 19 0.9 interest
We changed our minds; targetting 0.9.1 now.
Target Milestone: mozilla0.9 → mozilla0.9.1
Whiteboard: Apr 19 0.9 interest
I attached another patch file that has changes to the Win32 implementation of 
nsOSHelperAppService.  This does a number of things:

1. Removes the call that adds system-defined mime info entries to the cache.  
The cache entries have the problems enumerated in the "uriloader patch 
description file" I attached the other day.  The cost is some performance 
penalty (negligible in comparison to the cost of doing the subsequent 
network/disk I/O).  Note that this bug seems to also apply to the mac and os2 
implementations of this interface.  I will talk to Paul Chen about the mac and 
notify the OS2 code owner.

2. Adds a static utility function that sets additional nsIMIMEInfo fields from 
the contents of the registry.  It sets the Description to the default value 
under the "file type" key.  For example, HKCR/.ram has default 
value "RealPlayer.RAM.6" (or similar).  HKCR/RealPlayer.RAM.6->"RealPlayer 
File."  We now follow that chain to "RealPlayer File." Previously, the 
Description attribute was always blank.  This function also sets the 
alwaysAskBeforeHandling attribute according to the registry setting for 
HKCR/<filetype>/EditFlags (where <filetype> is the default value for the file 
extension, e.g., "RealPlayer.RAM.6").  This then causes the helper app dialog 
to not appear if the user has unchecked the "confirm open after download" 
checkbox in the Win32 Folder Options...File Types property sheet.

3. To enable proper handling for the EditFlags setting, I enhanced 
GetValueBytes to optionally return the length of the registry value.  The 
default argument value is 0 (which covers all the existing calls to this 
function).

Please examine this code if, by chance, you're helping out by (super-)reviewing 
the other uriloader related changes.
Updated the Win32 patch to properly free the buffer if the registry entry isn't 
of the normal/expected length.
Blocks: 78106
I am not sure how exactly it relates to this particular bug, but according to
bug #68009, in Linux Mozilla, the helper app _would not start at all_ unless
"always ask me" is enabled.
Blocks: 46655
With the checkins from 2001-05-03, Mozilla on Linux goes into an infinite loop
when one goes to http://www.google.com/mozilla/google.xul

We keep spawning more webshells...

The file uses a text/xul mimetype which we do not recognize, but a .xul
extension so we _can_ map it to the correct mimetype...
This seems to be a somewhat more pathological case.  I can't quite figure out
what's going on.

If you comment out the line at
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#248
which resets the mimetype when we get a match from the OS on extension, then it
doesn't spawn windows.  It opens the helper app dialog, but doesn't display the
xul.  How did it behave previously? 
Previously it would not display the XUL (since the XUL mimetype is not
text/xul).  It would show the helper app dialog and claim that it could not
handle application/vnd.mozilla.xul+xml

Note that the GetFromExtension() call in this case is never going out to the OS
(on Unix that's not supported yet).  What's actually happening is that we know
about the .xul extension internally, so Mozilla itself resets the mimetype.  I'm
not sure whether that's relevant -- I can't test a case when it _would_ go to
the OS, because that does not work on Unix...
OK... more raw data:

###!!! ASSERTION: NS_ENSURE_TRUE(window) failed: 'window', file
nsContentTreeOwner.cpp, line 576
###!!! Break: at file nsContentTreeOwner.cpp, line 576
###!!! ASSERTION: NS_ENSURE_TRUE(docShellElement) failed: 'docShellElement',
file nsXULWindow.cpp, line 938
###!!! Break: at file nsXULWindow.cpp, line 938
###!!! ASSERTION: NS_ENSURE_TRUE(windowElement) failed: 'windowElement', file
nsXULWindow.cpp, line 958
###!!! Break: at file nsXULWindow.cpp, line 958
###!!! ASSERTION: no xul:window: 'windowElement', file nsXULWindow.cpp, line 751
###!!! Break: at file nsXULWindow.cpp, line 751
WEBSHELL+ = 24

This happens over and over.

Breaking on this, it looks like we're trying to do the right thing:

#9  0x407cd3a1 in nsWindowWatcher::OpenWindowJS (this=0x8146a88,
aParent=0x8b68564, 
    aUrl=0x8b80580 "chrome://global/content/helperAppLauncher.xul", 
    aName=0x8b92790 "_blank", aFeatures=0x8b805d0 "chrome,titlebar", aDialog=1,
argc=1, 
    argv=0x8b7e3c4, _retval=0xbffff0bc) at nsWindowWatcher.cpp:543
#10 0x4067bcb1 in GlobalWindowImpl::OpenInternal (this=0x8b68560, cx=0x8b68748, 
    argv=0x8b7e3b8, argc=4, aDialog=1, aReturn=0xbffff180) at
nsGlobalWindow.cpp:3048
#11 0x406775c1 in GlobalWindowImpl::OpenDialog (this=0x8b68560, cx=0x8b68748, 
    argv=0x8b7e3b8, argc=4, aReturn=0xbffff180) at nsGlobalWindow.cpp:2128
#12 0x425046a0 in nsUnknownContentTypeHandler::Show (this=0x8b53400, 
    aLauncher=0x8b39ac4, aContext=0x8b67d88) at
nsUnknownContentTypeHandler.cpp:238
#13 0x4102d67f in nsExternalAppHandler::OnStartRequest (this=0x8b39ac0, 
    request=0x8b923e8, aCtxt=0x0) at nsExternalHelperAppService.cpp:859
#14 0x41020729 in nsDocumentOpenInfo::OnStartRequest (this=0x8b92398,
request=0x8b923e8, 
    aCtxt=0x0) at nsURILoader.cpp:242

Hmm..  More data. Looks like we go into an infinite loop as we try to load the
XUL file for the dialog from the jar.  Is the problem perhaps that we have now
associated a "wrong" mimetype with the XUL extension somehow?  And that as a
result we can't bring up the dialog properly?

One would think that that mimeinfo object is not being used anywhere else...
Just when I thought I about had this code figured out...

I think what's happening (or I should say, my interpretation of what's
happening; you're assessment seems to be pretty much on the money) is this:

The line of code I inserted is changing the "cached" entry for .xul so that it
has mime type text/xul, rather than text/vnd....  After we decide we can't
handle text/xul, we then open a .xul from a jar file, which wants to go figure
out the mimetype.  That lookup now uses the corrupted cache entry, which maps
.xul to text/xul, which isn't "xul", so we decide to open the downloading dialog
*again*, which opens the .xul from the jar file, which maps .xul to text/xul, ad
nauseum.

The intent of that line is to "fix" mimeinfos that come from the OS in cases
where the OS thinks the extension maps to a different mimetype than the server
indicates.  There's a thorough explanation in the "detailed explanation"
attached to this bug.  Basically, this bug is why you previously saw the bogus
dialog box that claimed we couldn't handle text/vnd.mozilla.xul+xml (it should
read "text/xul").

Now I see that this same code path also is hit when we find an extension in the
cache.  In that case, we have to preserve the original mimeinfo (in the cache)
and create a copy (where we'll reset the mimetype to "text/xul").

I'll work on that.  Thanks much for your assistance, Boris.
I'm going to back out that one line of code.  I think that we need to clone
nsIMIMEInfos fetched from the cache to avoid them getting changed.
I was just thinking about this.... Why are we unconditionally doing the
extension lookup?  I feel that we should only do an extension lookup if we have
no mimetype or if the mimetype is application/octet-stream.  Otherwise we're
overriding a valid server mimetype with something we guess...

This infinite recursion issue needs to be resolved anyway, since we need the
extension-based code for properly loading file:// urls if nothing else.  But it
would be good to get the other behavior right as well.
Boris, "unconditionally doing the extension lookup" problem is documented by bug
#46655.
That patch adds a new utility method to look up entries in the mime info cache
but clone them on return.

This stops the infinite recursion of dialogs.  We get the one assertion from
nsExternalAppHandler::RetargetLoadNotifications.  Then the dialog appears.  But
the dialog doesn't work because there is no "context" passed to the
nsHelperAppDlg object.

For some reason, nsExternalHandlerAppService::DoContent is passed a null
"context" in this scenario.  That's coming from the uriloader "kernel" which I
don't understand.  I've got to talk to mscott about this.
I've opened a new bug 78943 to track the issue with cloning cache entries,
resetting the mimetype in the mime info when matching by extension, and figuring
out what's up with text/xul.
Changing summary so I can mark this one fixed.

I've opened 3 new bugs:

bug 78943 to track a problem peculiar to text/xul and .xul files
bug 79539 to cover the fact that the "always ask me" checkbox is still missing 
from the helper app dialog
bug 79543 to address the fact that the dialog doesn't redisplay properly after 
opening a secondary dialog via the "Set Default..." button

Other helper app (dialog) problems should be tracked via new bugs.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Summary: Enable "always ask me" option on Downloading... dialog → Fix Downloading... dialog and general helper app stuff
Wait a second! The new bugs that you have opened are talking about the Windows
dialog. This bug used to be about the Linux dialog.

I am currently using Mozilla 0.9 on RedHat Linux 6.2 and the "always ask me"
option is still always selected and greyed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Aleksey, relax. The was checked in _after_ the 0.9 branch.  It is _not_ in 0.9. 
It _is_ in nightly build 2001-05-08-14 on Linux.  And the checkbox you speak of
is not just greyed out -- it is completely missing.  And that's covered by bug
79539.

resolving fixed again -- we use this same dialog on _all_ systems.
Status: REOPENED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
*** Bug 79624 has been marked as a duplicate of this bug. ***
Hi, with the current dialog that was just implemented, it seems like there's no
way to set "download" as a default action.  You can only set helper apps as
defaults.  I think we should be able to set download as a default too.
*** Bug 80609 has been marked as a duplicate of this bug. ***
new helper app dialog is in. New bugs are open for other issues..marking this 
puppy VERIFIED.
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: