183890 - submit values' button don't get submitted after javascript checks

Status: VERIFIED FIXED

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

Description

[interzone]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021130
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021130

In page
http://mail.interzone.gr/mozdemo.html
there are three submit buttons

Submit2 directly submits the form. After pushing that button, you may see the
location bar that writes the full query_string (it's a "get" for that reason).
The submit's name is also included (testmsg=Submit2)

Submit1 and Submit3 both call a javascript which checks if at least one message
is selected, asks for confirmation, and then submits the form.
Unfortunately, the form gets submitted without the submit's name.

The javascript is in http://mail.interzone.gr/sc/jscripts.js

You may of course press the buttons on will (there is no script behind that)


Reproducible: Always

Steps to Reproduce:
1. select at least one checkbox
2. press "submit1"
3. press "ok" on the popup js dialog
4. Watch the location bar to see the full query_string
5. go back
6. press submit2
7. Watch the location bar to see the full query_string, correct this time

Actual Results:  
The "submit" name/value pair is not submitted

Expected Results:  
The "submit" name/value pair should be submitted as on the button that does not
go through the js script

Checked with Konqueror 3.0.0, works all right
Checked with Netscape Navigator 4.8 (Linux), works all right
A friend checked with some IE (I thinks it was 5), works all right.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Pressing submit1/submit3 does not submit at all, in either Mozilla or Netscape
4.  This is probably a duplicate of bug 138957, but hard to test given the lack
of submission.  ;)

Comment 2

[interzone]

Hi
I just checked again. It DOES submit, but you must have been tricked from the
check for selected messages.
Click on any one of the checkboxes, press submit1, ok on the js popup and then
it submits.
If you wish, I will remove that check from the js

Comment 3

[Boris Zbarsky [:bzbarsky]]

Ah.  My apologies.  I missed step #1 in the steps to reproduce....

This is _not_ fixed in Linux trunk build 2002-12-04-22.  Over to Form Submission.

Comment 4

[Alexandru Savulov]

on it

Comment 5

[Alexandru Savulov]

this regressed in with the patch for bug 147878 ( i didn't thought about event
bubbling - my foult).

Comment 6

[Alexandru Savulov]

Attachment: the patch that corrects the wrong behaviour - needs to land on the 1.3a branch too

i thought i can save there a packed bool but i was wrong, i forgot about event
bubbling. damn!

Comment 7

[Alexandru Savulov]

wait i have an idea, maybe i can do somthing to save a packed bool. need to see
if i can check if i'm in an onSubmit event in the form control.

Comment 8

[Alexandru Savulov]

Attachment: the better patch for the regression onbug 147878 - needs to land on 1.3a too

there you go. this was the mistake I made. waht was I thinking? be cause of
event bubbling the flag is to be reset only if we are in the form's onSubmit.

Comment 9

[Alexandru Savulov]

Comment on attachment 108520 [details] [diff] [review]
the better patch for the regression onbug 147878 - needs to land on 1.3a too

this was my mistake, didn't thought about it. please review!

Comment 10

[John Keiser (jkeiser)]

Comment on attachment 108520 [details] [diff] [review]
the better patch for the regression onbug 147878 - needs to land on 1.3a too

eek!

Comment 11

[Alexandru Savulov]

Comment on attachment 108520 [details] [diff] [review]
the better patch for the regression onbug 147878 - needs to land on 1.3a too


i missed one check so event bubbling was canceling the flag at the wrong moment
- my mistake.

Comment 12

[Boris Zbarsky [:bzbarsky]]

> +  /** Whether the submission is to be defered be cause is from a script*/

"because" is one word

Comment 13

[Alexandru Savulov]

Attachment: the better patch for the regression onbug 147878 now with corrected comment

Comment 14

[Alexandru Savulov]

Comment on attachment 108524 [details] [diff] [review]
the better patch for the regression onbug 147878 now with corrected comment

transfering jkeiser's review

Comment 15

[Alexandru Savulov]

Comment on attachment 108524 [details] [diff] [review]
the better patch for the regression onbug 147878 now with corrected comment

boris,
i asked jst for sr= be cause he sr'ed the previous patches too, but if you want
you can do that too. thanks for the hint.

Comment 16

[Alexandru Savulov]

hmmm, there is something about this "be caue" i keep using that space there. not
intentionally though. :-)

Comment 17

[Alexandru Savulov]

Comment on attachment 108524 [details] [diff] [review]
the better patch for the regression onbug 147878 now with corrected comment


the patch on bug 147878 caused this regression so this one needs to go in.

Comment 18

[interzone]

Although I've lost you (sorry, I'm not a programmer in that level) I think that
I have to mention that:
there are at least 5 ways to do exactly the same thing (and all of these must work).

a) use the onSubmit to the form.
b) use the onClick to a submit buton (input type=submit onclick...)
c) use the onClick to a button buton (input type=button onclick...)
d) use the onClick to an image buton (input type=image onclick...)
e) use the onClick to a plain buton (button onclick...)

I'm not sure your solution handles the last three ones (again, I'm not a
programmer, sorry if I add overhead with that comment).

BTW I couldn't think of a workaround. Is there any?

Thanks 
Michalis.

Comment 19

[Brendan Eich [:brendan]]

Comment on attachment 108524 [details] [diff] [review]
the better patch for the regression onbug 147878 now with corrected comment

>-  /** Whether the submission was triggered by an Image or Submit*/
>+  /** Whether the submission is to be defered in case a script triggers it*/

"deferred", not "defered"

Space before */ please.

Fix these nits and a=[e-mail] for 1.3a checkin -- do it soon, we're
trying to wrap 1.3a up.

/be

Comment 20

[Kevin McCluskey (gone)]

-> jkeiser

Comment 21

[Alexandru Savulov]

oops, this on was fixed log time ago (2002 dec 9)
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsHTMLFormElement.cpp&root=/cvsroot&subdir=mozilla/content/html/content/src&command=DIFF_FRAMESET&rev1=1.130&rev2=1.131

Comment 22

[Vladimir Ermakov]

verifying on linux 2003-02-12-10-trunk