Closed Bug 132183 Opened 19 years ago Closed 16 years ago
Chrome registry can't handle uppercase package names
1.20 KB, application/vnd.mozilla.xul+xml
1.19 KB, application/vnd.mozilla.xul+xml
1.20 KB, application/octet-stream
1.20 KB, application/x-xpinstall
1.19 KB, application/x-xpinstall
2.79 KB, patch
|Details | Diff | Splinter Review|
1.78 KB, patch
|Details | Diff | Splinter Review|
Hi, I created an XPI package that simply pops up a window and says hello. When all of my file names where in lower case, everything worked. But when I changed the name of some of the files to mixed upper/lower case, it installed ok but when I tried to run it I received a pop-up window that said: The file /D:/Program%20Files/mozilla.org/Mozilla/chrome/packages/core/sayhello.xul cannot be found. Please check the location and try again. Here are the two XPI packages I created: WORKS: sayhi.xpi sayhi.jar install.js -content subdirectory sayhi.xul contents.rdf DOESN'T WORK: sayHello.xpi sayHello.jar install.js -content subdirectory sayHello.xul contents.rdf After submitting the original bug I think it is possible to upload files. If so, I'll upload my two XPI files. Thanks, David
There isn't anything different between this package and the other one except the file names (plus I changed the pointers to them in the code).
Is the application type on the attachments correct? When I click on either one of them with Mozilla, I get an error: XML Parsing Error: not well-formed Location: http://bugzilla.mozilla.org/attachment.cgi?id=75090&action=view Line Number 1, Column 3:PK --^
Can't seem to find the right MIME type to let people download the xpi from the bug report.
I figured it out now - I just let the web page decide the mime type.
Sorry, I'm not real organized today. The command I used to start the packages were as follows: mozilla.exe -chrome chrome://sayHello/content mozilla.exe -chrome chrome://sayhi/content
Is this an xpidl issue or an xpinstall issue? Error message appears to be coming from xpinstall.
You could be right that it is xpinstall. I'm not really sure.
David: thanks, I'm able to run both of the last two attachments, of type application/x-xpinstall. However, I can't find where the files are being saved to! I searched everywhere for sayhi.jar, sayhi.xul, for example, but I can't find them! I'm probably making some mistake, but note the installer is not asking me where to put the files, and doesn't tell me where they were saved. Let me reassign this one to Installer:XPI Packages for now. cc'ing self -
Assignee: dbradley → dveditz
Component: xpidl → Installer: XPI Packages
QA Contact: pschwartau → ktrina
I can confirm the bug, but everything looks good on the installer end. I can't just click on the links above, I have to save them locally with a .xpi extention, and then launch them browsing around using file:///c|/tmp/ etc. Looking at install.log shows they both install OK as far as the installer can tell. The install scripts use the "DELAYED_CHROME" style, and the stuff written to installed-chrome.txt is correct. Starting the browser appears to register the correct stuff in chrome.rdf Using the command-line or entering a chrome: url in the location bar works for the all-lowercase one, doesn't for the "sayHello" name. The error message shows a leading slash (wrong on windows) and a path that differs from the registered path. Since Mozilla itself has xul files with mixed case names it must be the package name (in contents.rdf, not the .jar filename) that's causing the problem.
Assignee: dveditz → trudelle
Status: UNCONFIRMED → NEW
Component: Installer: XPI Packages → XP Apps
Ever confirmed: true
QA Contact: ktrina → paw
Summary: XPI packages with capital letters can't be found → Chrome registry can't handle uppercase package names
Assignee: trudelle → hewitt
--> Jimmy is this your area?
QA Contact: paw → jimmylee
Close enough to be mine. :-)
*** Bug 257997 has been marked as a duplicate of this bug. ***
The dupe is Linux, so setting OS->All. Triggering this bug completely hoses Firefox in some cases, so it would be nice if the installPackage method at least threw an exception when installing such bad extension (so that the extension manager canceled the extension installation and reported the problem).
OS: Windows 2000 → All
I had similar problems. Editing chrome.rdf to make package name all lowercase allowed chrome to work.
I think this is caused by the standard URL parser thinking the <component> in chrome://<component>/content is a domain name and converts it to lowercase. This bug should probably should be in the Networking component
Tested - VSS_DocLinks_0.0.1.2.xpi from http://vssdoclinks.mozdev.org/ (changes: content\vssdoclinks -> content\vssDocLinks + affected links corrected and max-ver bump): - FF 1.0.4 loads frozen (same under safemode) - Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050527 Firefox/1.0+ extension installed without any errors but beside entry in EM it is not registered at all. The bug might be fixed in 2 steps: 1. If chrome can't handle uppercase package names then the extension should have been rejected by EM and alert with information shown to the user. 2. Add support in chrome for uppercase package names BTW is the product / component really the proper one ?
Darin/Neil, what do you think? Should/can we allow mixed-case package names? If not, I can pretty easily add JSconsole warnings when people try to register them (at least on the toolkit chromereg side), or I can just automatically lowercase the packagename during registration.
Assignee: hewitt → benjamin
Component: XP Apps → XRE Startup
Product: Mozilla Application Suite → Toolkit
Or maybe we should just implement a proper chrome URL class...
Other than nsStandardURL normalizes to lowercase I don't see why we need to switch away from it: perhaps we can just add a flag on standardurl that controls case-normalization (or we could just document that packages are always lowercase).
My vote would be for automatic lowercasing, along with documentation stating that package names are not case sensitive.
> My vote would be for automatic lowercasing, along with documentation stating > that package names are not case sensitive. Agreed.
Comment on attachment 205662 [details] [diff] [review] Normalize chrome packages to lowercase, rev. 1 >Index: chrome/src/nsChromeRegistry.cpp >+ rv = aChromeURL->GetHost(package); >+ NS_ENSURE_SUCCESS(rv, rv); > >+ if (EnsureLowerCase(package)) >+ aChromeURL->SetHost(package); The string code has a ToLowerCase method that returns void. Perhaps it would be better to make that version do what you want. Or, are you worried about making that version slower?
Attachment #205662 - Flags: second-review?(darin) → second-review+
Comment on attachment 205662 [details] [diff] [review] Normalize chrome packages to lowercase, rev. 1 >+static PRBool inline? >+EnsureLowerCase(char &ch) I suppose it would be too confusing to copy the names from nsURLHelper.cpp, but you might want to copy the implementation. >+ if (EnsureLowerCase(package)) >+ aChromeURL->SetHost(package); nsStandardURL already does this, that's why we have this bug in the first place.
Attachment #205662 - Flags: first-review?(neil.parkwaycc.co.uk) → first-review-
I copied this impl from nsReadableUtils.cpp
Attachment #205729 - Flags: first-review?(neil.parkwaycc.co.uk)
Comment on attachment 205729 [details] [diff] [review] Normalize chrome packages to lowercase, rev. 2 >+static PRBool >+EnsureLowerCase(char *aBuf) ... >+ EnsureLowerCase(package); The return value is redundant, no? Is this worth sprinkling inline on?
Attachment #205729 - Flags: first-review?(neil.parkwaycc.co.uk) → first-review+
Fixed on trunk. I also normalized skin/locale registrations, which I forgot in the original patch.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
so why not just call ToLowerCase?
> so why not just call ToLowerCase? Is there a version of ToLowerCase that takes a |char*| argument?
oh, hm. maybe you could call ToLowerCase(nsDependentCString(package)); ?
> oh, hm. maybe you could call ToLowerCase(nsDependentCString(package)); ? If that compiles then we have a problem: ToLowerCase(nsDependentCString("string literal"));
Actually, ToLowerCase(nsDependentCString("foo")) ends up modifying a copy of "foo". So, it would not have the desired affect that bsmedberg wants (i.e., it would lower case a copy of |package| instead).
(In reply to comment #31) >so why not just call ToLowerCase? For the same reason nsStandardURL doesn't?
*** Bug 324860 has been marked as a duplicate of this bug. ***
(In reply to comment #37) > *** Bug 324860 has been marked as a duplicate of this bug. *** > Thanks for that lightening fast response...however I still stand that URL naming conventions allow for mixed case. My vote is for MIXED CASE, even if it's a little bit late. So perhaps the next official release of Mozilla Firefox, could fix this particular feature of the '"standard URL parser * Neil Deakin'
Component: XRE Startup → Startup and Profile System
QA Contact: jimmykenlee → startup
You need to log in before you can comment on or make changes to this bug.