59035 - PDT+,[FIX]type of a BUTTON should default to submit

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P2)

Description

[robbe]

HTML 4.01 says:

<!ATTLIST BUTTON
[...]
  type (button|submit|reset) submit -- for use as form button --

In the Nov 1 nightly, <button type="submit"> will submit, while <button> will do
nothing at all. Both should be equivalent.

Comment 1

[robbe]

Attachment: both buttons should take you to the (non-existing) "target" resource

Comment 2

[Boris Zbarsky [:bzbarsky]]

Confirming on Linux trunk build 2000110421.

Comment 3

[rods (gone)]

Patch:
Index: nsHTMLButtonControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsHTMLButtonControlFrame.cpp,v
retrieving revision 3.106
diff -u -r3.106 nsHTMLButtonControlFrame.cpp
--- nsHTMLButtonControlFrame.cpp	2000/10/28 22:14:50	3.106
+++ nsHTMLButtonControlFrame.cpp	2000/12/14 19:18:29
@@ -306,7 +306,7 @@
 PRBool
 nsHTMLButtonControlFrame::IsSubmit(PRInt32 type)
 {
-  if (NS_FORM_BUTTON_SUBMIT == type) {
+  if (NS_FORM_BUTTON_SUBMIT == type || NS_FORM_BUTTON_BUTTON) {
     return PR_TRUE;
   } else {
     return PR_FALSE;

Comment 4

[Eric Pollmann]

Won't this patch also make <button type=button> submit a form when clicked on?
I think that the only thing to change is if no type is specified then make type
be 'submit'.

I just tested, and our current behaviour is exactly the same as IE 4.x and 5.x's
behaviour with <button> - the default type is 'button', not 'submit'.  Can this
be corrected only for strict mode?

Comment 5

[rods (gone)]

This can easily be just a Standard mode thing. But I don't see much of a 
difference between setting the type for button's to NS_FORM_BUTTON_SUBMIT 
instead of leaving them NS_FORM_BUTTON_BUTTON. Could you explain your thiking a 
little more?

Plus, my patch was wrong, it should be:
  if (NS_FORM_BUTTON_SUBMIT == type || NS_FORM_BUTTON_BUTTON == type) {

Comment 6

[rods (gone)]

Oh, I re-read what you wrote and yes, you are right, make it submit only when 
the type=submit or when the type isn't specified.

Comment 7

[rods (gone)]

Attachment: new patch

Comment 8

[Kevin McCluskey (gone)]

Rod the diff you sent me to review looks exactly like the patch on 12-18

"Plus, my patch was wrong, it should be:"
  if (NS_FORM_BUTTON_SUBMIT == type || NS_FORM_BUTTON_BUTTON == type) {

I can not view your latest patch. Could you re-attach it so I can review it.
thanks.

Comment 9

[rods (gone)]

Attachment: new unscrambled patch

Comment 10

[Kevin McCluskey (gone)]

lastest patch looks good.
r=[e-mail]

Comment 11

[rods (gone)]

fixed

Comment 12

[robbe]

Not verified: "Submit1" in my attached testcase still does not submit. Tested
with build 2000122223 which has rods's fix.

How does this CompatibilityMode work?

Comment 13

[jerone]

Verified build 20010730T os:Linux7.1
*code mistake*
*testcase's button1 has no assignment (type) while button2 type='submit'*

Comment 14

[robbe]

> Verified build 20010730T os:Linux7.1

Can't comment on that. The bug is present in 0.9.3 on Linux. Will try 0.9.4 when
it's there.

> *testcase's button1 has no assignment (type) while button2 type='submit'*

Exactly, that's the whole point of this bug. See the HTML4 DTD.

Comment 15

[robbe]

Sigh. NOT fixed in 0.9.3 (my previous post mistakenly mentioned 0.9.*4*).

Comment 16

[rods (gone)]

The correct patch:

Index: nsHTMLButtonControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsHTMLButtonControlFrame.cpp,v
retrieving revision 3.124
diff -u -r3.124 nsHTMLButtonControlFrame.cpp
--- nsHTMLButtonControlFrame.cpp        2001/08/21 13:56:18     3.124
+++ nsHTMLButtonControlFrame.cpp        2001/08/27 18:05:15
@@ -330,7 +330,7 @@
     // and oif it wasn't explicitly then we we allow to be a submit
     if (NS_FORM_BUTTON_BUTTON == type) {
       nsAutoString type;
-      if (NS_CONTENT_ATTR_HAS_VALUE == mContent->GetAttr(kNameSpaceID_HTML, nsH
TMLAtoms::type, type)) {
+      if (NS_CONTENT_ATTR_HAS_VALUE != mContent->GetAttr(kNameSpaceID_HTML, nsH
TMLAtoms::type, type)) {
         return PR_FALSE;
       } else {
         return PR_TRUE;

Comment 17

[rods (gone)]

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

Comment 18

[Katsuhiko Momoi]

Some forms will become disabled with this bug. 
Nominating for PDT.

Comment 19

[kinmoz]

Since the HTML 4.01 spec says that the button type should be submit, shouldn't
we just fix this by backing out the patch in this bug that was checked in, and
doing something like this?


Index: nsHTMLButtonElement.cpp
===================================================================
RCS file: /cvsroot/mozilla/content/html/content/src/nsHTMLButtonElement.cpp,v
retrieving revision 1.76
diff -u -r1.76 nsHTMLButtonElement.cpp
--- nsHTMLButtonElement.cpp     2001/08/17 08:12:55     1.76
+++ nsHTMLButtonElement.cpp     2001/09/06 23:10:46
@@ -127,7 +127,7 @@

 nsHTMLButtonElement::nsHTMLButtonElement()
 {
-  mType = NS_FORM_BUTTON_BUTTON; // default
+  mType = NS_FORM_BUTTON_SUBMIT; // default
 }

 nsHTMLButtonElement::~nsHTMLButtonElement()

Comment 20

[Marc Attinasi]

I like kin's suggestion. Try that out, Rod, it is a lot simpler.

Comment 21

[rods (gone)]

Attachment: testcase

Comment 22

[rods (gone)]

Yes, removing the old patch and applying Kin's patch does the trick, I guess I 
didn't realize the default was suppose to be "submit" thanks, kin

Comment 23

[kinmoz]

Rod, you going to generate a new patch based on the changes in your tree?

Comment 24

[Kevin McCluskey (gone)]

Marking nsbranch+. 

Comment 25

[Marc Attinasi]

sr=attinasi for backing out the previous patch, and checking in Kin's suggested
patch from 2001-09-06 16:16

Comment 26

[Kevin McCluskey (gone)]

r=[e-mail] for backing out the previous patch, and checking in
Kin's suggested patch from 2001-09-06 16:16

Comment 27

[elaineking]

removed keyword nsbranch since it now has nsbranch+, per pdt mtg.

Comment 28

[Kevin McCluskey (gone)]

Updated status

Comment 29

[Jaime Rodriguez, Jr.]

check this bad boy in today.

Comment 30

[Jaime Rodriguez, Jr.]

Make this work as kin thinks it should, and the spec says . . .

Comment 31

[rods (gone)]

fixed on tip and branch

Comment 32

[Madhur Bhatia]

verified fixed on RedHat Linux 7.1 build ID : 2001-09-19-04-0.9.4