Jar protocol fails silently on file-not-found

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
Networking
P3
normal
RESOLVED FIXED
19 years ago
12 years ago

People

(Reporter: Mitchell Stoltz (not reading bugmail), Assigned: Waldo)

Tracking

Trunk
mozilla1.9alpha1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

Loading a jar: URL which references a file which does not exist (either a jar 
file that doesn't exist or a nonexistent entry in a valid jarfile) fails 
silently. Should notify the user of the file-not-found, just as with any bad URL.

Comment 1

19 years ago
This looks like the same issue as in bug 19073, "Missing file: URL gives a 
blank page display", NEW, M14. Quoting from that bug:
  >----- Additional Comments From rpotts@netscape.com  1999-11-30 15:15 -----
  >This looks like a webshell issue...  Someone needs to look at the result 
  >from OnEndDocumentLoad(...) and display an error page...

Comment 2

19 years ago
NOt a beta1 stopper--marking it M15.
Status: NEW → ASSIGNED
Target Milestone: M15
(Reporter)

Updated

19 years ago
Blocks: 32881

Updated

19 years ago
Target Milestone: M15 → M16

Comment 3

19 years ago
Not an M15 stopper, marking M16.  Gayatri, please add an expected completion
date to the status white board.  Thanks.

Comment 4

18 years ago
I think I should take this one now since I've changed this code around so much 
since Gayatri worked on it. Gayatri - take it back if you disagree.
Assignee: gayatrib → warren
Status: ASSIGNED → NEW
Keywords: beta2
Whiteboard: 2d

Updated

18 years ago
Keywords: nsbeta2

Updated

18 years ago
Keywords: nsbeta2
Whiteboard: 2d
Target Milestone: M16 → M18

Comment 5

18 years ago
Not critical for beta2 since we shouldn't be asking for unknown files during the 
startup process.

Comment 6

18 years ago
Mitch, did you fix this?
(Reporter)

Comment 7

18 years ago
Nope. When you try to load a nonexistant jar: URL it will either spin 
indefinitely or print "error loading URL..." to the console, but there is no 
visible indication to the user.

Comment 8

18 years ago
Warren, I am reassigning some bugs which you own over to me (dougt).  If you 
think that you will have time to address them please reassign them back to 
yourself.  This reassignment was suggested by choffman.
Assignee: warsome → dougt

Comment 9

18 years ago
Based on warren's 2000-05-04 comment, unsetting milestone.  
Target Milestone: M18 → ---

Updated

18 years ago
Keywords: helpwanted
Target Milestone: --- → Future

Comment 10

17 years ago
Can we drag this bug to mozilla0.9.1? This bug encourages everyone to get away 
with bad chrome, missing CSS files etc etc. That means that those of us who build 
with files, not jars, in chrome suffer all the crashes, and waste lots of time 
debugging them.

Comment 11

17 years ago
resetting milestone.  I will try to get to it as soon as I can...
Status: NEW → ASSIGNED
Target Milestone: Future → mozilla0.9.2

Comment 12

17 years ago
mass move, v2.
qa to me.
QA Contact: tever → benc

Comment 13

17 years ago
jar affected by lack of general error reporting.

Updated

17 years ago
Blocks: 19073

Comment 14

17 years ago
mass moving target milestone.  if there is time, i will try to pull things back
in.  email me if you think that this must be fixed for 0.9.2, preferably with a
patch. :-)
Target Milestone: mozilla0.9.2 → mozilla0.9.3

Updated

17 years ago
Target Milestone: mozilla0.9.3 → Future

Updated

17 years ago
Target Milestone: Future → mozilla0.9.5

Comment 15

17 years ago
simon, didn't you add a debug printf in case of reading a non existant file?  I
am seeing printfs in my debug build about errors reading files.

Comment 16

17 years ago
I think we should put a line in the JS console, in both debug and non-debug 
builds, if we fail to load a file (including XUL, CSS etc) from chrome.

We need to also ensure that we have similar error handling code paths for the jar 
and non-jar case (i.e. working error reporting in both).

Comment 17

17 years ago
patches happily accepted
Target Milestone: mozilla0.9.5 → Future
(Assignee)

Comment 18

13 years ago
*** Bug 163613 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 19

13 years ago
I have a working patch for this which doesn't implement comment 16's suggestion; it will be posted when bug 309296 is fixed.
Assignee: dougt → jwalden+bmo
Status: ASSIGNED → NEW
(Assignee)

Comment 20

12 years ago
Created attachment 232764 [details] [diff] [review]
Show the error page

Showing an error page requires that NS_ERROR_FILE_NOT_FOUND be thrown, not NS_ERROR_FILE_TARGET_DOES_NOT_EXIST; I've never been able to get an answer about what the difference between the two is.  If there's no difference, we could just add it to the NS_ERROR_FILE_NOT_FOUND if in docshell (and get rid of the current error-translating that the file protocol does so it doesn't demonstrate this bug); I want confirmation that not doing this (and converting the thrown error to the one that docshell wants) is the correct thing to do.

The GetPath --> GetSpec change is necessary so that loading the non-existent jar:http://foo/bar.zip!/baz displays "jar:http://foo/bar.zip!/baz" as the file that couldn't be found (as opposed to just "http://foo/bar.zip!/baz").  I think this was done for the typical case where the not-found was a file (and trimming "file://" is arguably more user-friendly); if it's not desired (or if we want a special-case if for !file.schemeIs("file")) I'd like to be told so during reviews.
Attachment #232764 - Flags: superreview?(bzbarsky)
Attachment #232764 - Flags: review?(darin)
Comment on attachment 232764 [details] [diff] [review]
Show the error page

The only documentation I see for NS_ERROR_FILE_TARGET_DOES_NOT_EXIST is that nsIFile::moveTo and copyTo throw it if the file you're moving or copying does not exist.  I don't see any docs for NS_ERROR_FILE_NOT_FOUND, other than that the file:// protocol handler returns it.

Given that, I think it's reasonable to just change the return value.

>Index: docshell/base/nsDocShell.cpp
>-        aURI->GetPath(spec);
>+        aURI->GetSpec(spec);

How about QI to nsIFileURL?  If that succeeds, get the file path.  If not, then get the spec.

Looks good with that done.

Updated

12 years ago
Attachment #232764 - Flags: review?(darin) → review+
(Assignee)

Comment 22

12 years ago
Created attachment 233170 [details] [diff] [review]
Use aURI.schemeIs("file") check to determine what to display

Per bz on IRC, comment 21 is sr=him with the changes; also, since nsResProtocolHandler QIs to nsIFileURL, we want a .schemeIs("file") check rather than a QI.

Will check this in when I next have time...
Attachment #232764 - Attachment is obsolete: true
Attachment #233170 - Flags: superreview+
Attachment #233170 - Flags: review+
Attachment #232764 - Flags: superreview?(bzbarsky)
(Assignee)

Comment 23

12 years ago
Patch checked in, marking FIXED.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Keywords: helpwanted
Target Milestone: Future → mozilla1.9alpha

Comment 24

12 years ago
(In reply to comment #20)
>I think this was done for the typical case where the not-found was a file (and
>trimming "file://" is arguably more user-friendly); if it's not desired (or if
>we want a special-case if for !file.schemeIs("file")) I'd like to be told so
>during reviews.
I wonder if there's a rfe filed for seeing the platform-specific file name...
You need to log in before you can comment on or make changes to this bug.