21686 - StdURL parsing code doesn't lower case scheme name

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Roy Keyes]

with the lastest win32 mozilla (13 dec 1999) the javascript based menus on
www.rice.edu work fine, but not under linux.  The menu display correctly upon
mouse over, but nothing happens with a click.

Comment 1

[Mike McCabe]

Sorry, I've sat on this bug for a little while...

Does this problem still occur with m12?  If not, could you close the bug?  If
so, could you please attach a specific, simple, small testcase (just pointing at
a website make it hard for us to identify the problem) and reassign this bug to
the DOM component?  Thanks -

Mike

Comment 2

[Mike McCabe]

I've reproduced this bug with a recent build.  Clicking on the menus does indeed
have no effect.  It's not clear where exactly the problem occurs: does the
sendmap function associated with the menu entries get called?  If it does, does
the assignment to document.location work?

rkeyes: it's a little difficult to debug javascript hosted on a remote site (and
I was unable to reproduce the page by saving the top 'index.html' file to my
local disk.)  The best way to help us get this bug fixed is to produce the
simplest possible testcase that demonstrates the problem.  Ideally, this would
be an .html file with just a few lines, demonstrating (for instance) that
javascript: urls don't see the right scope, or that document.location doesn't
work.

Reassigning to the DOM component, as this doesn't seem to be a problem with the
core JavaScript language.

Comment 3

[Chad House]

The problem seems to be the use of "Javascript:" rather than "javascript:" for
the AREA href's in the client side image maps. Using Linux x86 build 2000011308,
I get a "Javascript is not a registered protocol" alert when clicking on the
menus at www.rice.edu.

Attaching a simple testcase.

Comment 4

[Chad House]

Attachment: simple testcase

Comment 5

[ekrock's old account (dead)]

Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!

Comment 6

[vidur (gone)]

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

Comment 7

[vidur (gone)]

Warren, here's bug that I mentioned earlier. The StdURL parsing code doesn't 
lower case the scheme name. If the fix works for you, go ahead and check it in. 
Changing the bug summary to reflect the real problem.

Index: nsStdURL.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/base/src/nsStdURL.cpp,v
retrieving revision 1.38
diff -u -r1.38 nsStdURL.cpp
--- nsStdURL.cpp        2000/01/24 23:18:44     1.38
+++ nsStdURL.cpp        2000/01/27 02:56:28
@@ -308,6 +308,7 @@
         if (*(brk+1) == '/')
         {
             ExtractString(mSpec, &mScheme, (brk - mSpec));
+            ToLowerString(mScheme);

             if (*(brk+2) == '/') // e.g. http://
             // If the first colon is followed by // then its definitely a spec
@@ -549,6 +550,7 @@
             else // scheme:host...
             {
                 ExtractString(mSpec, &mScheme, (brk - mSpec));
+                ToLowerString(mScheme);
                 brk = PL_strpbrk(lastbrk, delimiters);
                 if (!brk) // its just scheme:host
                 {
@@ -758,6 +760,17 @@
     {
         *o_Dest = nsnull;
         return NS_OK;
+    }
+}
+
+void
+nsStdURL::ToLowerString(char* i_Src)
+{
+    if (i_Src) {
+        while (*i_Src) {
+            *i_Src = nsCRT::ToLower(*i_Src);
+            i_Src++;
+        }
     }
 }

Index: nsStdURL.h
===================================================================
RCS file: /cvsroot/mozilla/netwerk/base/src/nsStdURL.h,v
retrieving revision 1.15
diff -u -r1.15 nsStdURL.h
--- nsStdURL.h  2000/01/24 21:27:41     1.15
+++ nsStdURL.h  2000/01/27 02:56:28
@@ -70,6 +70,7 @@
     // Some handy functions
     nsresult DupString(char* *o_Dest, const char* i_Src);
     nsresult ExtractString(char* i_Src, char* *o_Dest, PRUint32 length);
+    void ToLowerString(char* i_Src);

 protected:
     char*       mScheme;

Comment 8

[Warren Harris]

I thought you said it was a problem finding the protocol dll if the scheme name 
wasn't lower cased. I would have suspected that the fix for that was in 
nsIOService::ExtractScheme (in conjunction with 
nsIOService::GetProtocolHandler).

Comment 9

[Warren Harris]

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

Comment 10

[Warren Harris]

Here's my fix: 

Index: nsIOService.cpp
===================================================================
RCS file: /cvsroot/mozilla/netwerk/base/src/nsIOService.cpp,v
retrieving revision 1.59
diff -c -r1.59 nsIOService.cpp
*** nsIOService.cpp	2000/01/27 08:57:12	1.59
--- nsIOService.cpp	2000/01/28 06:20:30
***************
*** 154,159 ****
--- 154,160 ----
      char buf[MAX_NET_PROGID_LENGTH];
      nsCAutoString progID(NS_NETWORK_PROTOCOL_PROGID_PREFIX);
      progID += scheme;
+     progID.ToLowerCase();
      progID.ToCString(buf, MAX_NET_PROGID_LENGTH);
  
      rv = nsServiceManager::GetService(buf, NS_GET_IID(nsIProtocolHandler), 
(nsISupports **)result);


This fixes finding the protocol DLLs, but I'm not sure if other things will 
break because all URL schemes are not lowercased by default. I'll check this in, 
and let Andreas decide whether to add Vidur's fix to his new URL parsing code.

Comment 11

[Warren Harris]

Fix checked in. 

Comment 12

[Warren Harris]

asdf

Comment 13

[Scott MacGregor]

This bug fix broke mail today because  we have url schemes that contain capital
letters!!! We may need to back this out so we can open the tree.

Comment 14

[Scott MacGregor]

We had to back this fix out in order to get the tree open this morning. so I'm
going to re-open this now. mailnews has protocols like mailboxMessage which have
uppercase letters in them. This current patch breaks those cases.

I wonder if the right fix is to make the service manager do case insenstivie
comparisons when looking up a progid?

Comment 15

[Andreas Otte]

I think it's best to move mailboxMessage to mailboxmessage. I remember a
discussion about the registry being case sensitive, so we need a simple way to
ensure that caseing doesn't count, and that's all lower case. Only
nsMailboxService and nsMsgLocalCID have to be changed (at least that's what I
did on the ANDREAS_URL_BRANCH). 

Comment 16

[Warren Harris]

Fix checked in again.

Comment 17

[Andreas Otte]

This one lokks fixed to me. The site works as well as the testcase

Comment 18

[Andreas Otte]

the javascript menus work fine

Comment 19

[Mike McCabe]

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

Comment 20

[Mike McCabe]

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

Comment 21

[Roy Keyes]

It seems this bug has recently reappeared.  Linux build 2000021808 has it.  Try
the test case on the bug's homepage (21686).
Roy

Comment 22

[Andreas Otte]

It still works for me, but I also have the fixes for bug 22859 in my tree. This
used to work without that fixes, but that may have changed. Bug 22859 is related
to the security manager which might have increased security recently. I suggest
fixing 22859 and again have a look.

Comment 23

[Andreas Otte]

I think this one is fixed on linux, no need to move to M15.

Comment 24

[Warren Harris]

What's the status of this now? Bug 22859 is now fixed.

Comment 25

[Andreas Otte]

Marking this one fixed, haven't seen this for ages, please reopen
[e-mail] if it still does not work for you.

Comment 26

[Roy Keyes]

Everything seems to work on this end now.  I think that my report of the bug
resurfacing earlier was just because the fix was taken out of the code and I was
unaware of that at the time.  Good job everybody!
roy

Comment 27

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/5f067b73fd54
[fenix] Issue https://github.com/mozilla-mobile/fenix/issues/21686: Move submitList calls into TabsAdapter
https://hg.mozilla.org/releases/mozilla-beta/rev/93169fdf01be
[fenix] Issue https://github.com/mozilla-mobile/fenix/issues/21686: Stop using internally stored tabs list in adapters