Closed Bug 19073 Opened 25 years ago Closed 23 years ago

Error: nonexistent files never reported as error

Categories

(Core :: DOM: Navigation, defect, P3)

Product:
Core
Component:
DOM: Navigation
Platform:
x86
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla0.9.6

People

(Reporter: raj, Assigned: rpotts)

References

(
URL
)

Details

(Keywords: meta, Whiteboard: [PDT-][rtm-])

Attachments

(2 files)

patch
610 bytes, patch
Details | Diff | Splinter Review
better patch
733 bytes, patch
Details | Diff | Splinter Review 
This is in the precompiled M11 Linux binary.

In Communicator 4.x, an error dialog appears, which is much nicer. Also,
a URL like file://asdfsa/ shows the / directory of localhost. Instead,
like 4.x, it should complain that it can't find the server asdfsa.

It doesn't make a difference whether I type the URL directly into the
location text box, or use the Open File menu (and type in a non-existent
file into the file dialog).

The text output looks like this (from first trying file:///asdf, then
file://asdfa:

Document file:///asdf loaded successfully
Document: Done (0.697 secs)
FindShortcut: in='file://asdf'  out='null'
failed to set the page title.
Document file://asdf/ loaded successfully
Document: Done (1.116 secs)
The other problem with this is that the "blank screen" shown for a missing file
is not added to the history, so when you go back, you go back to whatever you
were on *before* the the page that had the link to the missing file.
Assignee: leger → gagan
Component: Browser-General → Networking-Core
*** Bug 19957 has been marked as a duplicate of this bug. ***
Assignee: gagan → travis
Target Milestone: M13
This looks like a webshell issue...  Someone needs to look at the result from
OnEndDocumentLoad(...) and display an error page...
Resetting QA Contact.
Confirmed blank page and no error notification with 1999-12-08-08-M12
nightly binary on Windows NT 4.0sp3  Here is the relevant console output:
  >FindShortcut: in='file:///.html'  out='null'
  >Error: Can't load: file:///.html (8052e891)
  >Error loading URL file:///.html
  >Document: Done (0.661 secs)

The problem mentioned by dbaron in his 11/22/99 comment about [Back] not
working properly after the file:/// load failure looks very much like
bug 13329 "Generic problem with history - skips pages when going back" -
probably fixed now. At least, the history is working for me in this case,
and it does look like there is already a bug for this.
Bulk move of all Networking-Core (to be deleted component) bugs to new
Networking component.
Target Milestone: M13 → M14
Move to M14.  Do we nedd to fix this for beta 1?
*** Bug 22792 has been marked as a duplicate of this bug. ***
*** Bug 26010 has been marked as a duplicate of this bug. ***
*** Bug 26097 has been marked as a duplicate of this bug. ***
Nominating for beta1, because I think this is a basic UI issue.  Also marking
4.xP.
Keywords: 4xp, beta1
Whiteboard: [PDT-]
The MICROSOFT WINDOWS 98 builds also share this flaw.
OS: Linux → All
Move to M16 ...
Target Milestone: M14 → M16
*** Bug 40539 has been marked as a duplicate of this bug. ***
*** Bug 42652 has been marked as a duplicate of this bug. ***
*** Bug 33132 has been marked as a duplicate of this bug. ***
M16 has been out for a while now, these bugs target milestones need to be 
updated.
M15:  
Problem relayed by David Baron (1999-11-22) - blank screen
No longer happens  (previous page continues to be displayed)

There is still no error message when a file is not found (as in Comm 4.x).
Adding mostfreq keyword, and adding "finger" to summary (see dup bug 33132).
Keywords: mostfreq
Summary: Missing file: URL gives a blank page display → Missing file: or finger: URL gives a blank page display
Nominating for rtm as a mostfreq (hoping... ;-) We really should have this. How 
come this bug fell through the net?

Gerv
Keywords: rtm
adding myself to the cc list.

valeski, you want me to investigate this?
mscott, do you own a duplicate of this?
Are we really getting huge numbers of complaints from beta users about this?
Since the fix for this requires a new dialog and it's too late for UI changes,
I'm marking this rtm-.  (Since there isn't already a patch, it's not likely to
make the train in any case.)
Whiteboard: [PDT-] → [PDT-][rtm-]
Setting milestone to something more sane than "M16". Valeski, if you have better 
thoughts on this, please change it again :-)

Gerv
Target Milestone: M16 → mozilla0.9
I didn't notice this one before - ftp doesn't report errors either. Neither does
gopher (not in the tree yet) or about: (Although about prints a couple of
NS_ENSURE_TRUE assertions). Retitling, and nominating for moz 0.9

about: may be separate - the others all bring up the directory tree componant.
Keywords: mozilla0.9
Summary: Missing file: or finger: URL gives a blank page display → Only http scheme reports errors
*** Bug 60421 has been marked as a duplicate of this bug. ***
*** Bug 60421 has been marked as a duplicate of this bug. ***
-> gagan
Assignee: valeski → gagan
http bugs to "Networking::HTTP"
Assignee: gagan → darin
Component: Networking → Networking: HTTP
Target Milestone: mozilla0.9 → M19
This isn't Networking:HTTP this is Networking:anything-but-HTTP :)
Component: Networking: HTTP → Networking
Keywords: beta1 → nsbeta1
this really applies to all protocols, it just happens that with HTTP, the server
reports many of the errors by simply sending some HTML content to the client.
this makes it appear as though HTTP is handling errors, when really it is doing
nothing more than displaying content from the server.

in general, error reporting from necko, needs much attention. 
> this really applies to all protocols, it just happens that with HTTP, the
> server reports many of the errors by simply sending some HTML content to the
> client.

Use mozilla to go to http://invalid.foo. There's a pop-up telling you that the
domain doesn't exist. This popup doesn't appear with ftp, or gopher or datetime,
or finger. ftp and gopher use the directory viewer, so that could be hijacking
it, but datetime and finger don't, so there goes that explanation :) I'm not
using a proxy.

This may be a separate bug though.

Just for the record, using TestProtocols for http (or gopher) on an invalid
domain prints out the fact that there was a DNS error, while this doesn't occur
for ftp:

./TestProtocols http://invalid ftp://invalid gopher://invalid
Warning: MOZILLA_FIVE_HOME not set.
Warning: MOZILLA_FIVE_HOME not set.
Type Manifest File: /home/bbaetz/src/mozilla/dist/bin/components/xpti.dat

Trying to load:
	http://invalid
WARNING: No disk cache present, file nsCacheManager.cpp, line 179
	ftp://invalid
	gopher://invalid

Finished loading: http://invalid  Status Code: 804b001e
	HTTP Status: 1073797506
	DNS lookup failed.
	Read: 0 bytes.
	Time to connect: 981657289.636 seconds
	Time to read: -981657289.493 seconds.
	Throughput: REAL FAST!!

Finished loading: ftp://invalid  Status Code: 0
	Read: 0 bytes.
	Time to connect: 981657289.672 seconds
	Time to read: -981657289.445 seconds.
	Throughput: REAL FAST!!

Finished loading: gopher://invalid  Status Code: 804b001e
	DNS lookup failed.
	Read: 0 bytes.
	Time to connect: 981657289.680 seconds
	Time to read: -981657289.340 seconds.
	Throughput: REAL FAST!!
CanUnload_enumerate: skipping native

ftp has a return status code of 0, which seems wrong
removed stale rtm keyword.
set target milestone = mozilla0.9
Keywords: rtm
Target Milestone: --- → mozilla0.9
dougt: do you have another bug tracking this for FTP?
Darin, there isn't one bug on this specifically.  However, I believe that I have
already fixed this in the currect changes that I am working on.  

If you like, you can assign this bug to me and I will see that ftp and file both
get fixed.  Or maybe it is better to spawn a bunch of bugs with block this bug?


Doug, yes.. let's make this a meta bug.  I'll leave it to you to add the 
appropriate blocker bugs for ftp:// and file://.
Since this applies to all of necko, I'm pushing the target milestone back to
mozilla1.0.  Dougt has other bugs tracking this problem for file:// and ftp://
with more aggressive targets.
Target Milestone: mozilla0.9 → mozilla1.0
*** Bug 77065 has been marked as a duplicate of this bug. ***
*** Bug 76434 has been marked as a duplicate of this bug. ***
mass move, v2.
qa to me.
QA Contact: tever → benc
Blocks: 77635
+meta.
changed summary for searchability. This seems to affect lots of areas.

jar affected in bug 26496.
sidebar affected in bug 77635.
No longer blocks: 77635
Depends on: 26496
Keywords: meta
Summary: Only http scheme reports errors → Error: nonexistent files never reported as error
Blocks: 31094
This has regressed yet further: there's no feedback now for http URLs either.
E.g. go to http://mozillaHasNoErrorFeedback.com -- it spins on "resolving host"
then changes to "Document: Done" showing a blank page and no error message.
(With a 6/25 build.)  Or is this http problem covered by a different bug?
Whoops, turns out that was specific to the 6/25 build; in 6/27 the dialog is
back for http urls.
yes, this is a different bug which has been fixed. please try a more recent build.
*** Bug 89423 has been marked as a duplicate of this bug. ***
Blocks: 90976
Blocks: 89118
Blocks: 63048
Blocks: 63878
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → mozilla0.9.4
Blocks: 95997
Blocks: 33132
-> docshell, necko is producing NS_ERROR_FILE_NOT_FOUND in the file channel's
onStopRequest.
Component: Networking → Embedding: Docshell
-> reassign
Assignee: darin → adamlock
Status: ASSIGNED → NEW
QA Contact: benc → adamlock
-> 0.9.5 sorry, we can't be dealing w/ existing regressions at the moment.

looks like we just need to respond to file-not-found.
Target Milestone: mozilla0.9.4 → mozilla0.9.5
*** Bug 100307 has been marked as a duplicate of this bug. ***
Entering a duff URL such as file:///c|fdsf/sdfsdfsd appears to partially work.
It generates the correct error message in an alert box but the content area is
displays "<html><body></body></html>" as text.
I see that (empty html source in the content area) fairly frequently for http
urls that do exist.  It's sporadic, and hitting reload usually gives me the real
page.  I don't often see it for nonexistant file urls.  I think it's an
unrelated problem, don't know whether there's a bug filed or not.
I have a patch. It seems like the right thing to do, but I'd like confirmation.
Attached patch patch — — Splinter Review 
No longer blocks: 89118

    
    

    
  
oh boy :-)  This opens up a very old can of worms :-)

I believe the current behavior of the FileChannel is due to an (outdated?)
assumption that OnStopRequest *must* be paired with OnStartRequest.

I'm pretty sure that this assumption is violated elsewhere... and i'm not even
sure if necko still publically has this requirement...

If pairing OnStart/OnStop is no longer necessary, then the attached patch looks
correct... Otherwise, I think that we'll need to fix the problem upstream, by
Calling GetStatus(...) and checking for failure... in some stream listener
implementation...

i'm cc'ing darin to get the 'official' word on whether OnStart/OnStopRequest
*must* be paired or not...

--rick



    
    

    
  
I think that they must be paired, and in fact they are.

What was happening was that the onstart was succeding, so we tore away the
document. Then we got the onstop, and popped up the dialog box, and got
about:blank for our troubles.

Now the onstart fails, so something (uriloader? docshell?) doesn't remove the
document, and the onstop checks the error.

Thats a guess - I haven't traced the code. Does that sound right, though?

    
    

    
  
OnStartRequest must be paired with an OnStopRequest ... no exceptions!

r=darin on the patch... i forgot to add that line when i added the code to
inherit mStatus from the underlying file transport.  thanks for fixing this :-)

    
    

    
  
taking.

darin: actually, now that I look at the code again, no other protocol does that
:) Whats special about file channels?
Assignee: adamlock → bbaetz

    
    

    
  
the error is known by the time OnStartRequest is fired, so we should make sure
the channel's status is correctly up-to-date.  this same idea applies to all
channels... even if it is not yet implemented (similarly) elsewhere.

    
    

    
  
Comment on attachment 50320 [details] [diff] [review]
patch

actually, on second thought, i don't understand how this patch helps with anything.  
returning failure from OnStartRequest causes the request to be canceled.
however, the request should already be "canceled" since there was a file transport error.

we need to better understand why this fix helps, because it can't be the correct fix.

    
    

    
  
darin and I looked at this, and I've come up with a uriloader patch.

mscott: you checked the original version of this code in way back when - why did
you only check for one specific error code (see the patch)
Status: NEW → ASSIGNED

    
    

    
  
i'm not convinced that this is correct... mscott: please advise.

    
    

    
  
No, this breaks ftp, possibly as a side effect of bug 101128, or we could just
not be updating the status.

I think its the right thing to do though.

    
    

    
  
ProcessCanceledCase cuts off the m_targetStreamListener, making the
OnStopRequest basically a nop.  this means that the code which handles
UNKNOWN_HOST in nsWebShell wouldn't get called, right??

    
    

    
  
But it works! Try it an see.

Without my patch, though, I don't see how we avoid destroying the current page
for an dns failure with http. mscott? rpotts?

Also note that http is the only protocol to get the error stuff right, and it
only has connection level errors. This does not fill me with confidence that
copy-and-paste of existing code is the right thing to do.

Is there a primer on how webshell/docshell/uriloader/necko are meant to interact?

    
    

    
  
OK, now that the duplicate-on-stop patch has gone in for ftp, this is good.
_and_ the dns errors for ftp, which load about:blank currently, as for file are
fixed by this.

I am now convinced that this is the correct thing to do. However, I'd really
like some feedback as to why this was done in the first place. Please?
Keywords: patch, 
          
            review
Target Milestone: mozilla0.9.5 → mozilla0.9.6

    
  
Blocks: 104166
Blocks: 100816

    
    

    
  
rick: bbaetz's patch should look very familar to you :)

    
    

    
  
-> rpotts
Assignee: bbaetz → rpotts
Status: ASSIGNED → NEW

    
    

    
  
This is basically the same patch as bug #106558 :-)

i'll leave it open until i land that patch, though -- so i don't lose all of the
blocking bugs...

-- rick

Depends on: 106558

    
    

    
  
I've been trying to get review on this for almost 5 weeks now from mscott. (I
mailed you on Friday, but you probably haven't got it yet, given what I've been
hearing about the network update)

I still haven't got an answer to the "why was it done this way in the first
place" question....

    
    

    
  
welcome to the club :-)

the code was originally added to fix bug #40116...  however, over time it has 
been scaled back.  first, the call to ProcessCanceledCase() was removed from 
OnStopRequest(...).

i believe that the call to ProcessCanceledCase() in OnDataAvailable(...) is 
unnecessary because it should always be safe to call the consumer with data - 
even if an error has occurred...

that just leaves the call within OnStartRequest(...) :-)  

i bet that the original patch was scoped very narrowly to just fix the immediate 
problem (back then NS_BINDING_ABORTED was about the only error code that could 
be passed to OnStartRequest)...

so i believe that short-circuiting on any FAILURE in OnStartRequest(...) is safe 
too...

-- rick


    
    

    
  
I've checked the patch in for bug #106558.  So this bug should be fixed too.

Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
Is this going to fix all the bugs that are marked blocked, or do those modules
still have to implement error messages?

    
    

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

    
    

    
  
adam: I can qa this if you want.

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: