113174 - value of document.forms[0].method is mixed case (inconsistent with both IE *and* NS4)

Status: VERIFIED FIXED

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

Description

[Jacob Kjome]

Using build 2001120108 on Win2k (SP2)

Mozilla is returning a mixed case "Post" and "Get" when using:
document.forms[0].method

Where Both IE5.5 and NS4 return lowercase "post" and "get"

I could not find anything in the DOM spec to state what the correct thing to 
do. However, if this isn't a w3c "standards" issue, then I think the standard 
behavior defaults to the precedent set by makers of past browsers.

The problem reared its ugly head for me when I was using a javascript library 
that checked whether the form was to be posted or sent as a get request by 
doing:
if (document.forms[0].method == "post") { ... }

Both IE5.5 and NS4 pass that check if the method was indeed set to post in the 
form's "method" attribute (no matter the case in the attribute).  However, 
Mozilla was failing and I couldn't figure out why until I did an alert() of the 
value of the method attribute: "Post" which, obviously, won't match "post" with 
javascript's case sensitivity.

Now, maybe the author should have done:
if (document.forms[0].method.toLowerCase() == "post") { ... }

However, in the past, there was no reason for this, so a lot of scripts just 
assume the value will be all lowercase.  I think Mozilla should change to use 
all lowercase or else it risks breaking existing scripts.

jake

Comment 1

[Jacob Kjome]

Attachment: testcase showing mixed case values being returned for document.forms[0].method

Comment 2

[Jacob Kjome]

This bug may apply to the values returned by other attributes as well.  Barring 
some specification to the contrary, I think they should all be returned in 
lowercase, assuming that is what both IE and NS4 consistently do.  So, don't 
just fix the "method" value returned and assume this issue is done.  All values 
returned by the DOM should be evaluated as to whether their case is proper and 
consistent with any spec and/or previous browser convention.

Jake

Comment 3

[Boris Zbarsky [:bzbarsky]]

The issue is that EnumValueToString defaults to |aFoldCase = PR_TRUE|.  So the
fix for form method would be to just fix nsHTMLFormElement::AttributeToString to
pass in PR_FALSE.

I have no idea whether any other properties need adjusting... I tried testing a
few, but I can't seem to come up with a way to access the elements (tables and
the like) in the NS 4.x DOM.  If someone could write a testcase for IE and
Mozilla, that would be great... (I have no IE to test with).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix for .method

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 60123 [details] [diff] [review]
Fix for .method

sr=jst

Comment 6

[Johnny Stenback (:jst)]

bz, feel free to take this bug from me if you check it in :-) And thanks for the
fix.

Comment 7

[Fabian Guisset]

Comment on attachment 60123 [details] [diff] [review]
Fix for .method

r=fabian

Comment 8

[Boris Zbarsky [:bzbarsky]]

checked in.  Leaving open to investigate other uses of this stuff.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: inner frame for testcase, with a list of what Mozilla, IE5.5, IE6, and NS4 return

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Real testcase (previous is inner frame)

Comment 11

[Boris Zbarsky [:bzbarsky]]

OK.  The verdict is that IE returns everything in all-lowercase.  This is a
distinct spec violation in at least one case (input type) but we follow IE and
Netscape 4 on that one in spite of the spec violation.

jst, are there any other things tested in that testcase that the spec says
_should_ be case-folded?

Comment 12

[Johnny Stenback (:jst)]

The next version of the DOM HTML spec will say that input.type should be all
lower case, the DOM HTML spec was wrong, for unknown reasons wrt the
capitalization of input.type. I can't actually think of any cases where we'd
want to capitalize attribute values.

Comment 13

[Boris Zbarsky [:bzbarsky]]

So...  Should I just wipe out the case-fold option and the associated logic in
EnumValueToString?  :)  Or just take out the default value and make all callers
pass in PR_FALSE?

Comment 14

[Johnny Stenback (:jst)]

Good question, unless we can find a real use for the argument (and I can't think
of one) then take it out.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Well... that testcase exercises all users of the method.  :)  Will take the
argument out.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to fix all occurrences

Fabian, jst what do you think?

Comment 17

[Johnny Stenback (:jst)]

Comment on attachment 60268 [details] [diff] [review]
Patch to fix all occurrences

Use CopyASCIItoUCS2() in stead of aResult.Assign(NS_Convert...) in
nsGenericHTMLElement::EnumValueToString().

It seems like the diffs for nsGenericHTMLElement.h are missing? Is that the
only missing thing?

Before checking this in, we should make sure that we're not breaking
nsHTMLLIElement, which used the uppercase folding code, and we should make sure
no other code used this method and expected to pass in true through the default
parameter. Did you check all those cases?

If so, sr=jst with the above fixed.

Comment 18

[Fabian Guisset]

Comment on attachment 60268 [details] [diff] [review]
Patch to fix all occurrences

r=fabian, yay, more cleanup!

Comment 19

[Boris Zbarsky [:bzbarsky]]

Yeah... looks like that .h is the only missing thing.  The CVS/Entries in that
dir got wacky... Fixed now.

And yes, I did check all those cases.  In every single case where we case-folded
IE5.5 and IE6 do not do so....

Comment 20

[Boris Zbarsky [:bzbarsky]]

checked in

Comment 21

[Jacob Kjome]

Wow!  Thanks for the quick action on this one.  Nice Job :-)

Jake

Comment 22

[Jerry]

the patch is missing something.
since it change EnumValueToString function in .cpp, but it doesnot change 
function defination in .h.


but anyway, it works well in current trunk.

Comment 23

[Fabian Guisset]

See comment #19, it explains the missing .h file

Comment 24

[Sivakiran Tummala]

marking verified, regarding the change required in the w3 specs it is a
completely different issue