2010-06-04 Dimitri Glazkov <dglazkov@chromium.org>
authordglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jun 2010 15:56:29 +0000 (15:56 +0000)
committerdglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jun 2010 15:56:29 +0000 (15:56 +0000)
        Reviewed by Darin Adler.

        Remove side effects of form submission and prepare FormDataBuilder for splitting up.
        https://bugs.webkit.org/show_bug.cgi?id=40184

        Refactoring, covered by existing tests.

        * html/HTMLFormElement.cpp:
        (WebCore::HTMLFormElement::prepareFormSubmission):
            * Changed to use new accessors on FormDataBuilder;
            * Simplified the logic around action URL;
            * Removed form submission side effect of element's enctype property being updated when
                submitting a mailto form;
            * Removed unnecessary updating of action URL for mailto forms.
        (WebCore::HTMLFormElement::submit): Moved action URL check into prepareFormSubmission.
        (WebCore::HTMLFormElement::parseMappedAttribute): Updated to use new methods on FormDataBuilder.
        * html/HTMLFormElement.h: Removed decls for isMailtoForm and dataEncoding methods;
            moved m_target and m_url to FormDataBuilder.
        * platform/network/FormData.cpp:
        (WebCore::FormData::appendDOMFormData): Removed unnecessary instantiation of FormDataBuilder.
        * platform/network/FormDataBuilder.cpp:
        (WebCore::FormDataBuilder::parseAction): Moved from HTMLFormControl.
        * platform/network/FormDataBuilder.h:
        (WebCore::FormDataBuilder::action): Ditto.
        (WebCore::FormDataBuilder::target): Ditto.
        (WebCore::FormDataBuilder::setTarget): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@61618 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/html/HTMLFormElement.cpp
WebCore/html/HTMLFormElement.h
WebCore/platform/network/FormData.cpp
WebCore/platform/network/FormDataBuilder.cpp
WebCore/platform/network/FormDataBuilder.h

index b9bb57a..1909de7 100644 (file)
@@ -1,3 +1,32 @@
+2010-06-04  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Remove side effects of form submission and prepare FormDataBuilder for splitting up.
+        https://bugs.webkit.org/show_bug.cgi?id=40184
+
+        Refactoring, covered by existing tests.
+
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::prepareFormSubmission):
+            * Changed to use new accessors on FormDataBuilder;
+            * Simplified the logic around action URL;
+            * Removed form submission side effect of element's enctype property being updated when
+                submitting a mailto form;
+            * Removed unnecessary updating of action URL for mailto forms.
+        (WebCore::HTMLFormElement::submit): Moved action URL check into prepareFormSubmission.
+        (WebCore::HTMLFormElement::parseMappedAttribute): Updated to use new methods on FormDataBuilder.
+        * html/HTMLFormElement.h: Removed decls for isMailtoForm and dataEncoding methods;
+            moved m_target and m_url to FormDataBuilder.
+        * platform/network/FormData.cpp:
+        (WebCore::FormData::appendDOMFormData): Removed unnecessary instantiation of FormDataBuilder.
+        * platform/network/FormDataBuilder.cpp:
+        (WebCore::FormDataBuilder::parseAction): Moved from HTMLFormControl.
+        * platform/network/FormDataBuilder.h:
+        (WebCore::FormDataBuilder::action): Ditto.
+        (WebCore::FormDataBuilder::target): Ditto.
+        (WebCore::FormDataBuilder::setTarget): Ditto.
+
 2010-06-22  Yuta Kitamura  <yutak@chromium.org>
 
         Reviewed by Alexey Proskuryakov.
index 58e7dc4..9d58934 100644 (file)
@@ -26,7 +26,6 @@
 #include "HTMLFormElement.h"
 
 #include "Attribute.h"
-#include "CSSHelper.h"
 #include "DOMFormData.h"
 #include "DOMWindow.h"
 #include "Document.h"
@@ -193,14 +192,6 @@ void HTMLFormElement::submitImplicitly(Event* event, bool fromImplicitSubmission
         prepareSubmit(event);
 }
 
-TextEncoding HTMLFormElement::dataEncoding() const
-{
-    if (isMailtoForm())
-        return UTF8Encoding();
-
-    return m_formDataBuilder.dataEncoding(document());
-}
-
 static void appendMailtoPostFormDataToURL(KURL& url, const FormData& data, const String& encodingType)
 {
     String body = data.flattenToString();
@@ -224,16 +215,19 @@ static void appendMailtoPostFormDataToURL(KURL& url, const FormData& data, const
 
 PassRefPtr<FormSubmission> HTMLFormElement::prepareFormSubmission(Event* event, bool lockHistory, FormSubmissionTrigger trigger)
 {
+    KURL actionURL = document()->completeURL(m_formDataBuilder.action().isEmpty() ? document()->url().string() : m_formDataBuilder.action());
+    bool isMailtoForm = actionURL.protocolIs("mailto");
+
     if (m_formDataBuilder.isPostMethod()) {
-        if (m_formDataBuilder.isMultiPartForm() && isMailtoForm()) {
-            // FIXME: This may fire a DOM Mutation Event. Do we really want this here?
-            setEnctype("application/x-www-form-urlencoded");
+        if (m_formDataBuilder.isMultiPartForm() && isMailtoForm) {
+            m_formDataBuilder.parseEncodingType("application/x-www-form-urlencoded");
             ASSERT(!m_formDataBuilder.isMultiPartForm());
         }
     } else
         m_formDataBuilder.setIsMultiPartForm(false);
 
-    RefPtr<DOMFormData> domFormData = DOMFormData::create(dataEncoding().encodingForFormSubmission());
+    TextEncoding dataEncoding = isMailtoForm ? UTF8Encoding() : m_formDataBuilder.dataEncoding(document());
+    RefPtr<DOMFormData> domFormData = DOMFormData::create(dataEncoding.encodingForFormSubmission());
     Vector<pair<String, String> > formValues;
 
     for (unsigned i = 0; i < m_associatedElements.size(); ++i) {
@@ -252,35 +246,25 @@ PassRefPtr<FormSubmission> HTMLFormElement::prepareFormSubmission(Event* event,
 
     RefPtr<FormData> formData;
     String boundary;
-    // FIXME: Figure out whether m_url.isNull check is necessary.
-    KURL actionURL = document()->completeURL(m_url.isNull() ? "" : m_url);
 
     if (m_formDataBuilder.isMultiPartForm()) {
         formData = FormData::createMultiPart(domFormData->items(), domFormData->encoding(), document());
         boundary = formData->boundary().data();
     } else {
         formData = FormData::create(domFormData->items(), domFormData->encoding());
-        if (m_formDataBuilder.isPostMethod() && isMailtoForm()) {
+        if (m_formDataBuilder.isPostMethod() && isMailtoForm) {
             // Convert the form data into a string that we put into the URL.
             appendMailtoPostFormDataToURL(actionURL, *formData, m_formDataBuilder.encodingType());
-            // FIXME: Why is setting m_url necessary here? This seems wrong if mail form is submitted multiple times?
-            m_url = actionURL.string();
-
             formData = FormData::create();
         }
     }
 
     formData->setIdentifier(generateFormDataIdentifier());
     FormSubmission::Method method = m_formDataBuilder.isPostMethod() ? FormSubmission::PostMethod : FormSubmission::GetMethod;
-    String targetOrBaseTarget = m_target.isEmpty() ? document()->baseTarget() : m_target;
+    String targetOrBaseTarget = m_formDataBuilder.target().isEmpty() ? document()->baseTarget() : m_formDataBuilder.target();
     return FormSubmission::create(method, actionURL, targetOrBaseTarget, m_formDataBuilder.encodingType(), FormState::create(this, formValues, document()->frame(), trigger), formData.release(), boundary, lockHistory, event);
 }
 
-bool HTMLFormElement::isMailtoForm() const
-{
-    return protocolIs(m_url, "mailto");
-}
-
 static inline HTMLFormControlElement* submitElementFromEvent(const Event* event)
 {
     Node* targetNode = event->target()->toNode();
@@ -402,9 +386,6 @@ void HTMLFormElement::submit(Event* event, bool activateSubmitButton, bool lockH
 
     if (needButtonActivation && firstSuccessfulSubmitButton)
         firstSuccessfulSubmitButton->setActivatedSubmit(true);
-    
-    if (m_url.isEmpty())
-        m_url = document()->url().string();
 
     frame->loader()->submitForm(prepareFormSubmission(event, lockHistory, formSubmissionTrigger));
 
@@ -438,9 +419,9 @@ void HTMLFormElement::reset()
 void HTMLFormElement::parseMappedAttribute(Attribute* attr)
 {
     if (attr->name() == actionAttr)
-        m_url = deprecatedParseURL(attr->value());
+        m_formDataBuilder.parseAction(attr->value());
     else if (attr->name() == targetAttr)
-        m_target = attr->value();
+        m_formDataBuilder.setTarget(attr->value());
     else if (attr->name() == methodAttr)
         m_formDataBuilder.parseMethodType(attr->value());
     else if (attr->name() == enctypeAttr)
index 7e3bd33..b19ff07 100644 (file)
@@ -136,8 +136,6 @@ private:
 
     void submit(Event*, bool activateSubmitButton, bool lockHistory, FormSubmissionTrigger);
 
-    bool isMailtoForm() const;
-    TextEncoding dataEncoding() const;
     PassRefPtr<FormSubmission> prepareFormSubmission(Event*, bool lockHistory, FormSubmissionTrigger);
     unsigned formElementIndex(HTMLFormControlElement*);
     // Returns true if the submission should be proceeded.
@@ -159,8 +157,6 @@ private:
     Vector<HTMLFormControlElement*> m_associatedElements;
     Vector<HTMLImageElement*> m_imageElements;
 
-    String m_url;
-    String m_target;
     bool m_autocomplete : 1;
     bool m_insubmit : 1;
     bool m_doingsubmit : 1;
index 431dbe4..9e4a227 100644 (file)
@@ -204,9 +204,8 @@ void FormData::appendFileRange(const String& filename, long long start, long lon
 
 void FormData::appendKeyValuePairItems(const BlobItemList& items, const TextEncoding& encoding, bool isMultiPartForm, Document* document)
 {
-    FormDataBuilder formDataBuilder;
     if (isMultiPartForm)
-        m_boundary = formDataBuilder.generateUniqueBoundaryString();
+        m_boundary = FormDataBuilder::generateUniqueBoundaryString();
 
     Vector<char> encodedData;
 
@@ -218,7 +217,7 @@ void FormData::appendKeyValuePairItems(const BlobItemList& items, const TextEnco
         ASSERT(key);
         if (isMultiPartForm) {
             Vector<char> header;
-            formDataBuilder.beginMultiPartHeader(header, m_boundary.data(), key->cstr());
+            FormDataBuilder::beginMultiPartHeader(header, m_boundary.data(), key->cstr());
 
             bool shouldGenerateFile = false;
             // If the current type is FILE, then we also need to include the filename
@@ -238,7 +237,7 @@ void FormData::appendKeyValuePairItems(const BlobItemList& items, const TextEnco
                 }
 
                 // We have to include the filename=".." part in the header, even if the filename is empty
-                formDataBuilder.addFilenameToMultiPartHeader(header, encoding, fileName);
+                FormDataBuilder::addFilenameToMultiPartHeader(header, encoding, fileName);
 
                 // If the item is sliced from a file, do not add the content type.
 #if ENABLE(BLOB_SLICE)
@@ -252,11 +251,11 @@ void FormData::appendKeyValuePairItems(const BlobItemList& items, const TextEnco
                     // MIME type of the generated file.
                     String mimeType = MIMETypeRegistry::getMIMETypeForPath(fileName);
                     if (!mimeType.isEmpty())
-                        formDataBuilder.addContentTypeToMultiPartHeader(header, mimeType.latin1());
+                        FormDataBuilder::addContentTypeToMultiPartHeader(header, mimeType.latin1());
                 }
             }
 
-            formDataBuilder.finishMultiPartHeader(header);
+            FormDataBuilder::finishMultiPartHeader(header);
 
             // Append body
             appendData(header.data(), header.size());
@@ -271,12 +270,12 @@ void FormData::appendKeyValuePairItems(const BlobItemList& items, const TextEnco
             if (encodedData.isEmpty() && key->cstr() == "isindex")
                 FormDataBuilder::encodeStringAsFormData(encodedData, stringValue->cstr());
             else
-                formDataBuilder.addKeyValuePairAsFormData(encodedData, key->cstr(), stringValue->cstr());
+                FormDataBuilder::addKeyValuePairAsFormData(encodedData, key->cstr(), stringValue->cstr());
         }
     }
 
     if (isMultiPartForm)
-        formDataBuilder.addBoundaryToMultiPartHeader(encodedData, m_boundary.data(), true);
+        FormDataBuilder::addBoundaryToMultiPartHeader(encodedData, m_boundary.data(), true);
 
     appendData(encodedData.data(), encodedData.size());
 }
index 13a457b..fb78b08 100644 (file)
@@ -25,6 +25,7 @@
 #include "config.h"
 #include "FormDataBuilder.h"
 
+#include "CSSHelper.h"
 #include "Document.h"
 #include "Frame.h"
 #include "FrameLoader.h"
@@ -48,6 +49,12 @@ FormDataBuilder::~FormDataBuilder()
 {
 }
 
+void FormDataBuilder::parseAction(const String& action)
+{
+    // FIXME: Can we parse into a KURL?
+    m_action = deprecatedParseURL(action);
+}
+
 void FormDataBuilder::parseEncodingType(const String& type)
 {
     if (type.contains("multipart", false) || type.contains("form-data", false)) {
index 390a87b..559906e 100644 (file)
@@ -45,12 +45,18 @@ public:
     bool isMultiPartForm() const { return m_isMultiPartForm; }
     void setIsMultiPartForm(bool value) { m_isMultiPartForm = value; }
 
+    const String& action() const { return m_action; }
+
+    const String& target() const { return m_target; }
+    void setTarget(const String& target) { m_target = target; }
+
     String encodingType() const { return m_encodingType; }
     void setEncodingType(const String& value) { m_encodingType = value; }
 
     String acceptCharset() const { return m_acceptCharset; }
     void setAcceptCharset(const String& value) { m_acceptCharset = value; }
 
+    void parseAction(const String&);
     void parseEncodingType(const String&);
     void parseMethodType(const String&);
 
@@ -72,6 +78,8 @@ private:
     bool m_isPostMethod;
     bool m_isMultiPartForm;
 
+    String m_action;
+    String m_target;
     String m_encodingType;
     String m_acceptCharset;
 };