No error checking for incorrect chrome reg

VERIFIED FIXED

Status

Core Graveyard
Installer: XPInstall Engine
P3
normal
VERIFIED FIXED
18 years ago
2 years ago

People

(Reporter: David Epstein, Assigned: David Hyatt)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
currently, registerChrome returns 0 for registering any folder regardless of 
whether it has a manifest.rdf. Expect to have errors returned for at least these 
invalid conditions: 
a) registering a folder not containing a manifest.rdf
b) pointing directly to a non-jar file (i.e. .txt file) and registering it.

I'm not sure about other conditions like using registerChrome(CONTENT, ...) to 
register skins. But at least the above two should be covered. XPInstall has 
error -239 defined for chrome registry error, but we need the error trapped in 
the chrome registration code in order to post it.

To see this:
1. go to http://jimbob/trigger3.html
2. Enter f_reg_chrome_file_error.xpi in URL field (http:/jimbob/jars/)
3. Press Trigger. Then OK.
4. Check logfile.
Result: registerChrome returns 0.
Expected: return -239.
(Reporter)

Comment 1

18 years ago
changed qa contact to depstein. also added test case f_reg_chrome_dir_error.xpi.

log file for f_reg_chrome_file_error.xpi:

-------------------------------------------------------------------------------
http://jimbob/jars/f_reg_chrome_file_error.xpi  --  06/23/2000 12:49:59
-------------------------------------------------------------------------------

     tests Chrome reg of content
     ---------------------------

     ** registerChrome of file should return error -239 = 0
     [1/2]	Installing: C:\Program Files\Netscape\0619\Netscape 
6\chrome\chromefile.txt
     [2/2]	Register Content: 
jar:file:///C:/Program%20Files/Netscape/0619/Netscape%206/chrome/chromefile.txt!
/

     Install completed successfully
     Finished Installation  06/23/2000 12:49:59
QA Contact: jimmylee → depstein
(Reporter)

Comment 2

18 years ago
Nominated for nsbeta3 by QA. This is a new feature for the benefit of install 
script writers. regChrome() *always* returns '0' for success even for error 
conditions, like pointing to a folder without a manifest.rdf file.
Keywords: nsbeta3
I can't return an error if the chrome registry doesn't, reassigning to hyatt.

I strongly doubt this would get plussed given his other bugs. This makes it 
hard for an install author to know what went wrong and debug his script if he 
messes up somewhere, but it doesn't prevent us from doing things right (unlike 
a crash, say).

Things an install author might do wrong we could detect:
 - pass an incorrect path (e.g. same as no manifest)
 - syntax error in manifest.rdf

The manifest.rdf could incorrectly describe the actual chrome present and there 
would be no way of detecting that except at run time. Probably getting the path 
wrong in the script would be the most common problem, the manifest problems 
would presumably be discovered during chrome development.

Yeah, I argue for "-" on this one.
Assignee: dveditz → hyatt

Comment 4

18 years ago
reassigning to warren per hyatt.
Assignee: hyatt → warren

Comment 5

18 years ago
I have the code for this.
Keywords: patch
Whiteboard: [nsbeta3+]

Comment 6

18 years ago
Created attachment 13172 [details] [diff] [review]
error handling code for chrome registry

Comment 7

18 years ago
I attached my patch... could someone give it a try?

Comment 8

18 years ago
I reviewed this with Hyatt the other day. Made a minor tweak and checked it in.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 9

18 years ago
Cross platform: Still returns 0, instead of error -239, for the 2 test cases I 
tried this with: pointing to a directory without a manifest file, and pointing 
directly to a non-jar file. Try http://jimbob/jars/f_reg_chrome_dir_error.xpi 
(no manifest file), and http://jimbob/jars/f_reg_chrome_file_error.xpi (non-jar 
file).

What error conditions does the patch cover? If it fixes other conditions, but 
not these 2, I could open up another bug just for these 2.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 10

18 years ago
I checked in all my error checking code, so if you're still seeing it, it's 
hyatt's bug.
Assignee: warren → hyatt
Status: REOPENED → NEW

Comment 11

18 years ago
removing nsbeta3+ for triage by new owners
Whiteboard: [nsbeta3+]
(Assignee)

Comment 12

18 years ago
Please open individual bugs for your specific errors and move the testcases to 
those bugs.

Thanks!
Status: NEW → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

18 years ago
done. submitted bugs 51769 and 51770 for the individual cases. Included URLs for 
each one.
Status: RESOLVED → VERIFIED
*** Bug 76338 has been marked as a duplicate of this bug. ***
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.