159328 - submit button doesn't work

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[Onur yalazı]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1b) Gecko/20020722
BuildID:    2002072204

The submit button pretends to be a mixeture of a button and a text box.
I can change the value of the text box. Also I can click the button where the
button doesn't overlapped by the text box. But it doesn't works.



Reproducible: Always
Steps to Reproduce:
1. create a html page including a form element with type SUBMIT (all capitals)
2. create a html page including a form element with type SUBMiT or submit (all
lowercase or just the i in lowercase)
3.

Actual Results:  if i open the first page it creates the bug
but if i open the other page it works fine.

Expected Results:  of course a working submit button

I use a redhat 7.2 with red carpet updates. 
and using Lang

PWD=/home/fsniper
XAUTHORITY=/home/fsniper/.Xauthority
WINDOWID=41943203
HOSTNAME=fsniper
LESSOPEN=|/usr/bin/lesspipe.sh %s
GDMSESSION=Default
GNOME_SESSION_NAME=Default
USER=fsniper
LS_COLORS=no=00:fi=00:di=01;34:ln=01;36:pi=40;33:so=01;35:bd=40;33;01:cd=40;33;01:or=01;05;37;41:mi=01;05;37;41:ex=01;32:*.cmd=01;32:*.exe=01;32:*.com=01;32:*.btm=01;32:*.bat=01;32:*.sh=01;32:*.csh=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.gz=01;31:*.bz2=01;31:*.bz=01;31:*.tz=01;31:*.rpm=01;31:*.cpio=01;31:*.jpg=01;35:*.gif=01;35:*.bmp=01;35:*.xbm=01;35:*.xpm=01;35:*.png=01;35:*.tif=01;35:
MACHTYPE=i386-redhat-linux-gnu
MAIL=/var/spool/mail/fsniper
INPUTRC=/etc/inputrc
XMODIFIERS=@im=none
LANG=tr_TR.ISO-8859-9
COLORTERM=gnome-terminal
DISPLAY=:0
LOGNAME=fsniper
SHLVL=3
GDM_LANG=tr_TR.ISO-8859-9
SESSION_MANAGER=local/fsniper:/tmp/.ICE-unix/4405
SHELL=/bin/bash
HOSTTYPE=i386
OSTYPE=linux-gnu
HISTSIZE=1000
HOME=/home/fsniper
SSH_ASKPASS=/usr/libexec/openssh/gnome-ssh-askpass
TERM=xterm
PATH=/bin:/usr/bin:/usr/bin/X11:/usr/local/bin:/usr/games:/opt/gnome/bin:/usr/X11R6/bin:/home/fsniper/bin:/home/fsniper/bin
_=/usr/bin/env

Comment 1

[Jonas Jørgensen]

Reporter: Please create an html page which shows the problem (as you described
in Steps to Reproduce) and attach it to this bug. Thanks.

Comment 2

[Wesha]

WFM 2002072508/win2000.

Testcase needed!!!

Comment 3

[Boris Zbarsky [:bzbarsky]]

reporter, please save the page as "web page, complete" and attach to this bug.

Comment 4

[Onur yalazı]

Attachment: a buggy html page

The buggy mixture is like a button on the edges. 
Clickable but not working

Comment 5

[Onur yalazı]

Attachment: an another buggy html page

Comment 6

[Onur yalazı]

Attachment: a working html page

Comment 7

[Onur yalazı]

Attachment: an another working html page

Comment 8

[Onur yalazı]

Comment on attachment 92967 [details]
an another buggy html page

it seems to be usable on the edges but it doesn't responce

Comment 9

[Onur yalazı]

Attachment: an industrial buggy page

Comment 10

[Alexandru Savulov]

the industrial buggy page attachment contains nested forms, in that case we have
to deal wiht tech evangelism. the rest of the test cases WFM with Mozilla 1.1b /
RH 7.1. If this is the case, please reassign to tech evangelism.

Comment 11

[Onur yalazı]

well i couldn't understand what you mean by saying ""WFM with Mozilla 1.1b /
RH 7.1" 
I'm using 

Mozilla 1.1b
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1b) Gecko/20020722

and i found out that if i run mozilla with LANG=en_US (in my case i use tr_TR)
the bug doesn't exists maybe i should go on using this way

Comment 12

[Alexandru Savulov]

WFM = WORKSFORME (see: [RADIO] Resolve bug changing resolution to [SELECT BOX]
under the Additional Comments box)

Comment 13

[Onur yalazı]

if it WFU try (if you can) at a console LANG=tr_TR;$PATHTOMOZILLA/mozilla
now it should produce this bug (or isn't it?)

Comment 14

[Boris Zbarsky [:bzbarsky]]

I just did just that:

env LANG=tr_TR mozilla

And I cannot reproduce the bug (Linux build from 2002-07-29-08).  Though I _did_
get the following on startup:

Gdk-WARNING **: locale not supported by C library

so that could be an issue.

Comment 15

[Onur yalazı]

cause i use locale tr_TR and most of Turkish users this is a bug. If you have
time could you please compile or instal a tr_TR system and try this bug it can
be really easy to get over.
may be just a mini-patch to html parser.
(sorry i have not read any code from mozilla)

maybe a "I" to "i" convert regexp (does the parser use regexps) ?

Comment 16

[Boris Zbarsky [:bzbarsky]]

The parser does not use regexps...

Heikki? jkeiser?  any ideas what could be going on here?

Comment 17

[Onur yalazı]

well in turkish locale we have I and İ (a capital case of i say I with a dot)
and also i and ı (a lower case of I i without the dot ).

This is really confusing sometimes especialy when using english localed softwares. 
the software libs doesn't know to handle these cases and they start to fail
working correctly. The problem is (i think) mozilla parser converts capitals to
lowercases and with tr locale it converts I to real tr locale one  (i without
the dot) 

it means not a bug! the libs works correctly. 
but the problem is there isn't any "submıt"  tag?! (i without the dot) so maybe
mozilla should assume there is a "submıt" tag (again i without the dot)

(you should make your mozilla fonts iso8859-9 to see them in a good shape i
think you can see an apostrophe  over y but this is not the case)

Comment 18

[Onur yalazı]

sorry i was just trying galeon when i wrote thi comment above. 
It made a trick i think :)
it convert ý (lower case I) to İ and Ý (upper case of i ) to  ı
may be it helps you to understand the case.

Comment 19

[John Keiser (jkeiser)]

Attachment: valign=cEnTeR testcase

Could you test this testcase and tell me if the box on the left has the text
vertically centered?

I haven't reproduced this (no Turkish stuff), but I am suspicious that
EqualsIgnoreCase may do different things for different languages.  That's what
we're using to check the type against "submit".  Which reminds me, we need to
use ParseEnumAttr to parse input type.

Comment 20

[Onur yalazı]

Attachment: how the locales are seen

Comment 21

[Onur yalazı]

Attachment: WFM

this can not be a case e and E aren't confusing letters i think no other cases
than
i to Ý and I to ý

Comment 22

[John Keiser (jkeiser)]

OK, let me get this straight ... you are saying that our parser converts the "I"
to "&#305"?  That does sound like a bug if so, at least when dealing with
programmatic values.  Nothing should get converted at all.

Comment 23

[Onur yalazı]

well as i told you i assumed that the parser converts all the strings in the
html to lower cases  (let say thought that )

Because when i look at buggy pages all have upper case type="SUBMIT" value. Also
they didn't have nested forms. (look bdn.borland.com doesn't have nested forms
as well. they close one and open the other)

So i tried the test cases i've written. the problem seems to be a locale problem.

if the parser uses just lower cases and uses std libs to convert uppers to lower
cases the problem is mozilla doesn't know a subm"ý"t entry for html type values.
in my locale "I" *must be* converted to "ý". So my libs are working well.

When i try "SUBMiT" or SuBmiT  or etc. this bug doesn't accurs. It occurs when i
try "submIt" "SuBmIT" etc...
is it clear now?  
I told you i don't know how mozilla parser works. Just the bug and my local
makes me think like that 

(well and because i study comp. sciences ) 

Comment 24

[Onur yalazı]

and i do not tell you that moz converts "I"'s to encrypted strings. Just saying
moz converts it to another letter "ý". which is encrypted with ı with w3c
standarts. 

Comment 25

[Boris Zbarsky [:bzbarsky]]

Right.  Got that.  ;)

Do you build by any chance?  That is, would you be able to test possible fixes
for this?

Comment 26

[Onur yalazı]

i would really like to but now i've got some jobs to do. In a few days i would
like to try.

but don't expect much :) moz is really huge in code.

Comment 27

[alge]

Shouldn't this be I18N?

Comment 28

[Onur yalazı]

this is up to you i think. If you think I18N is just translating the visible
parts        then this is not a I18N bug. But if I18N is more than this you may
be right ;)

can somebody help me using ddd? I'm stuck with sources...

Comment 29

[Onur yalazı]

I got a new bug . write a full line on this text box (it shouldn't get to next
line) press space as much as you like it doesn't works

Comment 30

[Boris Zbarsky [:bzbarsky]]

Let's keep new bugs separate.

Onur, could you answer John's question from comment 19?

Also, when you say the submit button does not work...

1) Does it look like a button?
2) Can you type in it?

For debugging on Linux, I recommend reading
http://www.mozilla.org/unix/debugging-faq.html

Comment 31

[Onur yalazı]

>> Let's keep new bugs separate.

Well right. That was wrong but i have just got it when writing comment 28

>> Onur, could you answer John's question from comment 19?

well sorry i thought I've answered it. 
it worked for me well.

>> Also, when you say the submit button does not work...

>> 1) Does it look like a button? 

yes. see description pls.

>> 2) Can you type in it? yes.

>> For debugging on Linux, I recommend reading
>> http://www.mozilla.org/unix/debugging-faq.html

thank you...

Comment 32

[Boris Zbarsky [:bzbarsky]]

OK.  Over to strings.

Onur, you sound like you have a tree... if you do, and have a build running, I
can try creating a patch to at least test our theory on what's going on here...
 Let me know, please.

Comment 33

[Onur yalazı]

i'm really sorry but i couldn't understand you. 
Is 'having a tree' an idiom? 
or do you mean i got a working build that should be made a patch?
if so i do not. But i've some ideas about the bug. i've watched getType();
somewhere (sorry i've losen it) but it turned 14 for the subm"I"t buton where it
should 13 maybe this helps us :( i'm sorry nothing more 

Comment 34

[Boris Zbarsky [:bzbarsky]]

What I meant was:

"If I made a patch, could you apply it to the Mozilla source and build with it,
so that you can test it?"  

To "have a tree" means to have a copy of the Mozilla source (which is a
directory tree under mozilla/, right?  ;) ) and have the compiler and so forth
set up so that you can build the browser.

Since you say you were watching getType(), sounds like you do.  ;)  And since it
came up 14, sounds like John is right in his guess of what's wrong...

Comment 35

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch for testing only; DO NOT CHECK THIS IN

Onur, could you apply this to your tree and see whether it fixes the bug for
you?

Comment 36

[Onur yalazı]

well I behaved like an idiot so :) 

of course i will compile with pleasure.

Comment 37

[Onur yalazı]

but how can i apply this to my "tree"?
:..(( 

Comment 38

[Boris Zbarsky [:bzbarsky]]

cd mozilla/content
patch -p0 < patchfile
make

(where "patchfile" is the filename of the file you saved the patch in)

Comment 39

[Onur yalazı]

[fsniper@fsniper mozilla]$ env LANG=tr_TR /usr/local/patchymoz/bin/mozilla
###!!! ASSERTION: NS_InitXPCOM failed: 'NS_SUCCEEDED(rv)', file nsAppRunner.cpp,
line 1788
Break: at file nsAppRunner.cpp, line 1788
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
###!!! ASSERTION: Setting main thread twice?: 'Error', file nsThread.cpp, line 401
Break: at file nsThread.cpp, line 401
/usr/local/patchymoz//lib/mozilla-0.9.9+/run-mozilla.sh: line 72: 12300 Paralama
arzas       $prog ${1+"$@"}
[fsniper@fsniper mozilla]$ 

 
isn't it a bit confusing?
parcalanma arizasi means segmentation fault

Comment 40

[Onur yalazı]

is it because this tree is 1.1.a?

Comment 41

[Boris Zbarsky [:bzbarsky]]

That _could_ do it.. but should not.

Could you back out my patch and build?  Does that start up fine, I assume?

The patch applied cleanly, right?  No error messages?

Comment 42

[Onur yalazı]

well sorry that was my fault i think..
i compiled my tree without the patch but this occured again. May be i need to
download a clean source or it's just because permissions

the patch was successful

Comment 43

[Boris Zbarsky [:bzbarsky]]

OK.  Apply the patch again, then do

make -f client.mk clobber
make -f client.mk build

from the top-level?  Make sure to back out any changes you've made to that source...

Comment 44

[Onur yalazı]

well the source should be clean ( I used the downloaded tar ball )

i'm doing this now:

[fsniper@fsniper mozilla]$ pwd
/home/fsniper/desktop/moz/mozilla
[fsniper@fsniper mozilla]$ cd content/ 
[fsniper@fsniper content]$ patch -p0 < ../../patch.txt 
patching file html/content/src/nsHTMLInputElement.cpp
Hunk #1 succeeded at 846 (offset -72 lines).
Hunk #2 succeeded at 1755 (offset -37 lines).
Hunk #3 succeeded at 1749 (offset -72 lines).
[fsniper@fsniper content]$ ./configure
--prefix=/home/fsniper/desktop/moz/patchedmoz 
[fsniper@fsniper content]$ make
[fsniper@fsniper content]$ make install

if this doesn't work i'll use yours compile parameters

Comment 45

[Boris Zbarsky [:bzbarsky]]

> [fsniper@fsniper content]$ ./configure

You need to do 

cd ..

before you do that...  

Comment 46

[Onur yalazı]

oh :) well right
what a fool!
any chance to erease these comments ?? :)

Comment 47

[Onur yalazı]

well there've been sometime. But i'm glad i'm back with good news :)
I've downloaded a new 1.0 tree,applied the patch and compiled it (just 5 minutes
ago) 
now these incorrect submit buttons work well.

Do you think is it soon to say patch is ok?

Comment 48

[Onur yalazı]

well a new thing to add
seems that new product is much slower :( 

i wish the problem is something else...

Comment 49

[Boris Zbarsky [:bzbarsky]]

Yes, it's too soon.  The patch is a gross hack... ;)

Confirming bug, ccing some people.  To summarize:

1)  In the TR locale, toupper('i') != 'I' and tolower('I') != 'i'.
2)  EqualsIgnoreCase perfoms uppercasing and lowercasing using the locale-aware
    tolower() system function
3)  This means that in the TR locale,
    NS_ConvertASCIItoUCS2("Input").EqualsIgnoreCase("input") tests _false_

jkeiser and I talked about this briefly, and one option we floated is a new
comparator that will take an nsAString and a nsACString (or char*) and do a
non-locale-aware case-insensitive comparison on them under the assumption that
both represent pure ascii strings.

Other thoughts>

Comment 50

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

EqualsIgnoreCase shouldn't be considering locale.  If it is, then we did
something wrong with |setlocale|.

(Maybe we should never have switched to tolower/toupper...)

Comment 51

[Boris Zbarsky [:bzbarsky]]

So... 

EqualsIgnoreCase calls EqualsWithConversion
                 calls CompareWithConversion
                 calls nsStrPrivate::StrCompare2To1
                 calls Compare2To1
                 calls tolower()

Nowhere in this callchain is setlocale() called that I can see...  Are we
supposed to be calling that "globally" or something?  Or is this code just
missing setlocale() calls somewhere in here (in CompareWithConversion, probably)?

Comment 52

[Alec Flett]

yeah, sounds like toupper/tolower is broken for this locale... I think we didn't
think about the fact that toupper/tolower would be locale-sensitive. back to
nsCRT::ToLower and friends?

Comment 53

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Well, |setlocale| (which sets the local for the whole app) is supposed to work,
if we call it correctly.  Are we?

(Then again, do we really want to trust plugins not to call it?  And
non-locale-sensitive toupper/tolower are easy enough to write.  And I've seen
some weird bugs in a gcc3 mach-o build that I think were related to
toupper/tolower...)

Comment 54

[Alec Flett]

sounds like homegrown toupper/tolower is what we want - 99 times out of 100,
EqualsIgnoreCase means "do an ASCII case-insensitive compare" because we're
comparing to a an ASCII-named HTML attribute or a well-known ASCII string like
"submit" - i.e. a locale-blind comparison.

Comment 55

[Onur yalazı]

well what about using an error correction algortihm on the very last stages 
of this procedure? 

EqualsIgnoreCase calls EqualsWithConversion
                 calls CompareWithConversion
                 calls nsStrPrivate::StrCompare2To1
                 calls Compare2To1
                 calls tolower()

maybe while comparing, using an levenstein dist 1 compares should work for
non-error free case conversion locales?

Comment 56

[Onur yalazı]

wow :) i've  started to forget english :)
an levenstein dist 1 compares !?

Comment 57

[Boris Zbarsky [:bzbarsky]]

We don't need to do anything complicated here, Onur.  Just not use the
locale-aware comparison functions.

Comment 58

[Alec Flett]

Attachment: use nsCRT::ToLower instead

ok, lets try this.

dbaron/jag want to review? this just switches us over to nsCRT::ToLower()

Comment 59

[Alec Flett]

over to me.

Comment 60

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 95573 [details] [diff] [review]
use nsCRT::ToLower instead

sr=dbaron

(It would be nice to remove the include of ctype.h at some point, but that
might require more testing...)

Comment 61

[Kathleen :Brade]

Comment on attachment 95573 [details] [diff] [review]
use nsCRT::ToLower instead

r=brade

Comment 62

[Alec Flett]

thanks folks, fix is in.

Comment 63

[Alec Flett]

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

Comment 64

[Alec Flett]

oops, that caused xpcom_glue bustage and I had to back myself out.. reopening.

Comment 65

[Alec Flett]

Attachment: use inline tolower

ok, this is a little cleaner - don't use nsCRT and instead just write a custom
ascii_tolower - can I get reviews again ? :)

Comment 66

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 95587 [details] [diff] [review]
use inline tolower

Does that work?  Aren't capitals the lower numbers?  (I would probably have
done aChar + ('a' - 'A') anyway.)

Comment 67

[Alec Flett]

Attachment: use inline tolower v1.1

gack, you're right. I much prefer the 'a' - 'A' method as well, but I was
stealing from the "Patch for testing only" thing above.. duh. 

Anyway, here's the right patch. I'll go actually test this now :)

Comment 68

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 95590 [details] [diff] [review]
use inline tolower v1.1

This seems fine (pending testing), although it might be nice to make it one big
condition:

if (aIgnoreCase && c1 < 128 && c2 < 128 &&
    ascii_tolower(c1) == ascii_tolower(c2))
  continue;

which would save the conversion from the char return back to PRUnichar.  (Will
compilers optimize that correctly?)

Comment 69

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 95590 [details] [diff] [review]
use inline tolower v1.1

Ignore my previous comment.  This is fine as-is and what I suggested would be
broken.

Comment 70

[jag (Peter Annema)]

Comment on attachment 95590 [details] [diff] [review]
use inline tolower v1.1

r=jag

Are these the only tolowers we need to fix? What about the ones in
nsAString.cpp?

Comment 71

[Alec Flett]

this got checked in earlier this week. We should file a new bug on the other
uses of tolower/etc..