Summary: | method/enctype/formMethod/formEnctype properties should be limited to known values | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kent Tamura <tkent> | ||||||
Component: | Forms | Assignee: | Kent Tamura <tkent> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, code.vineet, dglazkov, morrita, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Kent Tamura
2011-09-27 04:02:24 PDT
Created attachment 108950 [details]
Patch
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. 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.. (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. (In reply to comment #4) > I would be better. Thank you! I -> It Comment on attachment 108950 [details]
Patch
It looks introducing an enum similar to FormSubmission::Method can be good idea. Maybe it's overkill though.
Created attachment 109439 [details]
Patch for landing
Update methodString() for Vineet's comment
Comment on attachment 109439 [details] Patch for landing Clearing flags on attachment: 109439 Committed r96484: <http://trac.webkit.org/changeset/96484> All reviewed patches have been landed. Closing bug. |