Last Comment Bug 132183 - Chrome registry can't handle uppercase package names
: Chrome registry can't handle uppercase package names
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Startup and Profile System (show other bugs)
: Trunk
: x86 All
: P2 normal with 1 vote (vote)
: mozilla1.9alpha1
Assigned To: Benjamin Smedberg [:bsmedberg]
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
: 257997 324860 (view as bug list)
Depends on:
Blocks: 248125
  Show dependency treegraph
 
Reported: 2002-03-19 16:42 PST by David Barnes
Modified: 2008-07-31 03:00 PDT (History)
12 users (show)
benjamin: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
This XPI installs, but can't be run. See error message in bug report. (1.20 KB, application/vnd.mozilla.xul+xml)
2002-03-19 16:43 PST, David Barnes
no flags Details
This XPI file works fine. Notice there are no capital letters in file names. (1.19 KB, application/vnd.mozilla.xul+xml)
2002-03-19 16:44 PST, David Barnes
no flags Details
Try this one - different attachment type. (1.20 KB, application/octet-stream)
2002-03-19 17:20 PST, David Barnes
no flags Details
Trying one more time (1.20 KB, application/x-xpinstall)
2002-03-19 17:23 PST, David Barnes
no flags Details
The previous XIP was sayHello and showed the possible bug. This one does work. (1.19 KB, application/x-xpinstall)
2002-03-19 17:25 PST, David Barnes
no flags Details
Normalize chrome packages to lowercase, rev. 1 (2.79 KB, patch)
2005-12-12 13:48 PST, Benjamin Smedberg [:bsmedberg]
neil: first‑review-
darin.moz: second‑review+
Details | Diff | Splinter Review
Normalize chrome packages to lowercase, rev. 2 (1.78 KB, patch)
2005-12-13 08:14 PST, Benjamin Smedberg [:bsmedberg]
neil: first‑review+
Details | Diff | Splinter Review

Description David Barnes 2002-03-19 16:42:16 PST
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
Comment 1 David Barnes 2002-03-19 16:43:18 PST
Created attachment 75089 [details]
This XPI installs, but can't be run. See error message in bug report.
Comment 2 David Barnes 2002-03-19 16:44:49 PST
Created attachment 75090 [details]
This XPI file works fine. Notice there are no capital letters in file names.

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).
Comment 3 Phil Schwartau 2002-03-19 16:56:13 PST
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
--^
Comment 4 David Barnes 2002-03-19 17:20:01 PST
Created attachment 75095 [details]
Try this one - different attachment type.
Comment 5 David Barnes 2002-03-19 17:23:44 PST
Created attachment 75096 [details]
Trying one more time

Can't seem to find the right MIME type to let people download the xpi from the
bug report.
Comment 6 David Barnes 2002-03-19 17:25:17 PST
Created attachment 75097 [details]
The previous XIP was sayHello and showed the possible bug. This one does work.

I figured it out now - I just let the web page decide the mime type.
Comment 7 David Barnes 2002-03-19 17:31:06 PST
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
Comment 8 David Bradley 2002-03-19 17:39:07 PST
Is this an xpidl issue or an xpinstall issue? Error message appears to be coming
from xpinstall.
Comment 9 David Barnes 2002-03-19 17:56:18 PST
You could be right that it is xpinstall. I'm not really sure.
Comment 10 Phil Schwartau 2002-03-19 18:12:03 PST
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 -
Comment 11 Daniel Veditz [:dveditz] 2002-03-19 19:38:03 PST
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.
Comment 12 Peter Trudelle 2002-03-20 00:09:43 PST
->hewitt
Comment 13 Paul Wyskoczka 2002-03-20 14:40:38 PST
--> Jimmy is this your area?
Comment 14 Jimmy Lee 2002-03-20 15:06:49 PST
Close enough to be mine. :-)
Comment 15 Ted Mielczarek [:ted.mielczarek] 2004-11-30 11:25:39 PST
*** Bug 257997 has been marked as a duplicate of this bug. ***
Comment 16 Nickolay_Ponomarev 2004-11-30 13:03:59 PST
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).
Comment 17 Sean McMurray 2005-01-26 09:06:53 PST
I had similar problems. Editing chrome.rdf to make package name all lowercase
allowed chrome to work.
Comment 18 Neil Deakin 2005-01-26 09:08:30 PST
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
Comment 19 Przemyslaw Bialik 2005-05-29 11:35:40 PDT
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 ?
Comment 20 Benjamin Smedberg [:bsmedberg] 2005-11-16 09:16:04 PST
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.
Comment 21 neil@parkwaycc.co.uk 2005-11-16 13:11:30 PST
Or maybe we should just implement a proper chrome URL class...
Comment 22 Benjamin Smedberg [:bsmedberg] 2005-11-16 13:27:00 PST
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).
Comment 23 Ted Mielczarek [:ted.mielczarek] 2005-11-16 13:47:35 PST
My vote would be for automatic lowercasing, along with documentation stating that package names are not case sensitive.
Comment 24 Benjamin Smedberg [:bsmedberg] 2005-12-12 13:48:22 PST
Created attachment 205662 [details] [diff] [review]
Normalize chrome packages to lowercase, rev. 1
Comment 25 Darin Fisher 2005-12-12 16:24:39 PST
> My vote would be for automatic lowercasing, along with documentation stating
> that package names are not case sensitive.

Agreed.
Comment 26 Darin Fisher 2005-12-12 16:27:40 PST
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?
Comment 27 neil@parkwaycc.co.uk 2005-12-13 02:50:57 PST
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.
Comment 28 Benjamin Smedberg [:bsmedberg] 2005-12-13 08:14:09 PST
Created attachment 205729 [details] [diff] [review]
Normalize chrome packages to lowercase, rev. 2

I copied this impl from nsReadableUtils.cpp
Comment 29 neil@parkwaycc.co.uk 2005-12-13 17:31:58 PST
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?
Comment 30 Benjamin Smedberg [:bsmedberg] 2005-12-14 07:50:05 PST
Fixed on trunk. I also normalized skin/locale registrations, which I forgot in the original patch.
Comment 31 Christian :Biesinger (don't email me, ping me on IRC) 2005-12-18 07:42:40 PST
so why not just call ToLowerCase?
Comment 32 Darin Fisher 2005-12-18 10:47:06 PST
> so why not just call ToLowerCase?

Is there a version of ToLowerCase that takes a |char*| argument?
Comment 33 Christian :Biesinger (don't email me, ping me on IRC) 2005-12-18 12:05:04 PST
oh, hm. maybe you could call ToLowerCase(nsDependentCString(package)); ?
Comment 34 Darin Fisher 2005-12-18 12:13:59 PST
> oh, hm. maybe you could call ToLowerCase(nsDependentCString(package)); ?

If that compiles then we have a problem:

  ToLowerCase(nsDependentCString("string literal"));
Comment 35 Darin Fisher 2005-12-18 12:16:39 PST
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).
Comment 36 neil@parkwaycc.co.uk 2005-12-18 13:34:00 PST
(In reply to comment #31)
>so why not just call ToLowerCase?
For the same reason nsStandardURL doesn't?
Comment 37 Benjamin Smedberg [:bsmedberg] 2006-01-26 14:11:04 PST
*** Bug 324860 has been marked as a duplicate of this bug. ***
Comment 38 Martimus8 2006-01-26 14:54:09 PST
(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'

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