Bug 68887 - method/enctype/formMethod/formEnctype properties should be limited to known values
Summary: method/enctype/formMethod/formEnctype properties should be limited to known v...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-27 04:02 PDT by Kent Tamura
Modified: 2011-10-02 23:58 PDT (History)
5 users (show)

See Also:


Attachments
Patch (36.05 KB, patch)
2011-09-27 19:30 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch for landing (36.00 KB, patch)
2011-10-02 22:52 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2011-09-27 04:02:24 PDT
These IDL attributes are defined as "limited to only known values".

http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#dom-fs-method
http://www.whatwg.org/specs/web-apps/current-work/multipage/association-of-controls-and-forms.html#dom-fs-formmethod


How to see the problem:
1. Visit http://samples.msdn.microsoft.com/ietestcenter/html5/forms_harness.htm?url=formenctype4
2. You'll see the following tests fail:
  - verify default value for formEnctype
  - verify default value for formMethod
Comment 1 Kent Tamura 2011-09-27 19:30:38 PDT
Created attachment 108950 [details]
Patch
Comment 2 Kent Tamura 2011-09-27 19:33:21 PDT
Comment on attachment 108950 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108950&action=review

> Source/WebCore/loader/FormSubmission.h:63
> -        void parseMethodType(const String&);
> +        static Method parseMethodType(const String&);
> +        void updateMethodType(const String&);
> +        static String methodString(Method method) { return method == GetMethod ? "get" : "post"; }

I'm not satisfied with these function names. Any other ides?

> Source/WebCore/loader/FormSubmission.h:73
> -        void parseEncodingType(const String&);
> +        static String parseEncodingType(const String&);
> +        void updateEncodingType(const String&);

ditto.
Comment 3 Vineet Chaudhary (vineetc) 2011-09-28 00:31:48 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=108950&action=review

>> Source/WebCore/loader/FormSubmission.h:63
>> +        static String methodString(Method method) { return method == GetMethod ? "get" : "post"; }

I am not reviewer, but here are my observation:

though methodString gets its argument from parseMethodType() which will always return GetMethod for any invalid/null value but as this is static method having,
static String methodString(Method method) { return method == PostMethod ? "post" : "get"; }  would be much safer. 

Also just a suggestion can we have GET/POST instead of get/post.
//Sorry if I am wrong..
Comment 4 Kent Tamura 2011-09-28 01:27:43 PDT
(In reply to comment #3)
> though methodString gets its argument from parseMethodType() which will always return GetMethod for any invalid/null value but as this is static method having,
> static String methodString(Method method) { return method == PostMethod ? "post" : "get"; }  would be much safer. 

I would be better.  Thank you!

> Also just a suggestion can we have GET/POST instead of get/post.
> //Sorry if I am wrong..

No. The standard, Firefox 6, and IE8 normalize them to get/post, not GET/POST.
Comment 5 Kent Tamura 2011-09-28 01:29:15 PDT
(In reply to comment #4)
> I would be better.  Thank you!

I -> It
Comment 6 Hajime Morrita 2011-10-02 21:28:48 PDT
Comment on attachment 108950 [details]
Patch

It looks introducing an enum similar to FormSubmission::Method can be good idea. Maybe it's overkill though.
Comment 7 Kent Tamura 2011-10-02 22:52:28 PDT
Created attachment 109439 [details]
Patch for landing

Update methodString() for Vineet's comment
Comment 8 WebKit Review Bot 2011-10-02 23:58:32 PDT
Comment on attachment 109439 [details]
Patch for landing

Clearing flags on attachment: 109439

Committed r96484: <http://trac.webkit.org/changeset/96484>
Comment 9 WebKit Review Bot 2011-10-02 23:58:38 PDT
All reviewed patches have been landed.  Closing bug.