Changeset 35611 in webkit

Timestamp:

Aug 6, 2008 2:15:39 PM (16 years ago)

Author:

ddkilzer@apple.com

Message:

WebCore:

Bug 20038: REGRESSION (r35151): Can't post comments on flickr.com

<​https://bugs.webkit.org/show_bug.cgi?id=20038>
<rdar://problem/6092270>

Reviewed by Eric Seidel.

Test: fast/forms/submit-to-url-fragment.html

The problem was that isFormSubmission was not being set to true in
FrameLoader::loadWithDocumentLoader() when we were actually
submitting a form, causing the page to scroll instead of the form to
be submitted.

The isFormSubmission variable wasn't set to true because a FormState
object was not being created in
FrameLoader::loadFrameRequestWithFormAndValues().

The FormState object was not being created because
HTMLFormElement::submit(Event*, bool activateSubmitButton) would
only set FrameLoader::m_formAboutToBeSubmitted to the current form
if there was a "text field" element in the form (type = text,
password, search or isindex).

Thus when FrameLoader::submitForm(const FrameLoadRequest&, Event*)
called FrameLoader::loadFrameRequestWithFormAndValues(), a null
HTMLFormElement pointer would be sent and cause the above failures.

• html/HTMLFormElement.cpp: (WebCore::HTMLFormElement::submit): Call new FrameLoader::setFormAboutToBeSubmitted() method outside the for loop so we always set FrameLoader::m_formAboutToBeSubmitted exactly once for any form submission. The FrameLoader::recordFormValue() method is only called with the name/value pair of each text field.

• loader/FrameLoader.cpp: (WebCore::FrameLoader::setFormAboutToBeSubmitted): Added method that only sets m_formAboutToBeSubmitted. (WebCore::FrameLoader::recordFormValue): Removed PassRefPtr<HTMLFormElement> argument since this method only sets name/value pairs on m_formValuesAboutToBeSubmitted now. (WebCore::FrameLoader::loadFrameRequestWithFormAndValues): Create a FormState object as long as submitForm is not null so that other FrameLoader methods know when a form is being submitted.
• loader/FrameLoader.h:

LayoutTests:

Bug 20038: REGRESSION (r35151): Can't post comments on flickr.com

<​https://bugs.webkit.org/show_bug.cgi?id=20038>
<rdar://problem/6092270>

Reviewed by Eric Seidel.

• fast/forms/submit-to-url-fragment-expected.txt: Added.
• fast/forms/submit-to-url-fragment.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/submit-to-url-fragment-expected.txt (added)
• LayoutTests/fast/forms/submit-to-url-fragment.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/html/HTMLFormElement.cpp (modified) (2 diffs)
• WebCore/loader/FrameLoader.cpp (modified) (2 diffs)
• WebCore/loader/FrameLoader.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2008-08-05  David D. Kilzer  <ddkilzer@apple.com>
+
+        Bug 20038: REGRESSION (r35151): Can't post comments on flickr.com
+
+        <https://bugs.webkit.org/show_bug.cgi?id=20038>
+        <rdar://problem/6092270>
+
+        Reviewed by Eric Seidel.
+
+        * fast/forms/submit-to-url-fragment-expected.txt: Added.
+        * fast/forms/submit-to-url-fragment.html: Added.
+
 2008-07-02  David Kilzer  <ddkilzer@apple.com>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-08-05  David D. Kilzer  <ddkilzer@apple.com>
+
+        Bug 20038: REGRESSION (r35151): Can't post comments on flickr.com
+
+        <https://bugs.webkit.org/show_bug.cgi?id=20038>
+        <rdar://problem/6092270>
+
+        Reviewed by Eric Seidel.
+
+        Test: fast/forms/submit-to-url-fragment.html
+
+        The problem was that isFormSubmission was not being set to true in
+        FrameLoader::loadWithDocumentLoader() when we were actually
+        submitting a form, causing the page to scroll instead of the form to
+        be submitted.
+
+        The isFormSubmission variable wasn't set to true because a FormState
+        object was not being created in
+        FrameLoader::loadFrameRequestWithFormAndValues().
+
+        The FormState object was not being created because
+        HTMLFormElement::submit(Event*, bool activateSubmitButton) would
+        only set FrameLoader::m_formAboutToBeSubmitted to the current form
+        if there was a "text field" element in the form (type = text,
+        password, search or isindex).
+
+        Thus when FrameLoader::submitForm(const FrameLoadRequest&, Event*)
+        called FrameLoader::loadFrameRequestWithFormAndValues(), a null
+        HTMLFormElement pointer would be sent and cause the above failures.
+
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::submit): Call new
+        FrameLoader::setFormAboutToBeSubmitted() method outside the for
+        loop so we always set FrameLoader::m_formAboutToBeSubmitted exactly
+        once for any form submission.  The FrameLoader::recordFormValue()
+        method is only called with the name/value pair of each text field.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::setFormAboutToBeSubmitted): Added method that
+        only sets m_formAboutToBeSubmitted.
+        (WebCore::FrameLoader::recordFormValue): Removed
+        PassRefPtr<HTMLFormElement> argument since this method only sets
+        name/value pairs on m_formValuesAboutToBeSubmitted now.
+        (WebCore::FrameLoader::loadFrameRequestWithFormAndValues): Create a
+        FormState object as long as submitForm is not null so that other
+        FrameLoader methods know when a form is being submitted.
+        * loader/FrameLoader.h:
+
 2008-08-06  David D. Kilzer  <ddkilzer@apple.com>
 

trunk/WebCore/html/HTMLFormElement.cpp

@@ -444 +444 @@
     
     frame->loader()->clearRecordedFormValues();
+    frame->loader()->setFormAboutToBeSubmitted(this);
     for (unsigned i = 0; i < formElements.size(); ++i) {
         HTMLFormControlElement* control = formElements[i];
@@ -449 +450 @@
             HTMLInputElement* input = static_cast<HTMLInputElement*>(control);
             if (input->isTextField()) {
-                frame->loader()->recordFormValue(input->name(), input->value(), this);
+                frame->loader()->recordFormValue(input->name(), input->value());
                 if (input->isSearchField())
                     input->addSearchResult();

trunk/WebCore/loader/FrameLoader.cpp

@@ -1750 +1750 @@
 }
 
-void FrameLoader::recordFormValue(const String& name, const String& value, PassRefPtr<HTMLFormElement> element)
+void FrameLoader::setFormAboutToBeSubmitted(PassRefPtr<HTMLFormElement> element)
 {
     m_formAboutToBeSubmitted = element;
+}
+
+void FrameLoader::recordFormValue(const String& name, const String& value)
+{
     m_formValuesAboutToBeSubmitted.set(name, value);
 }
@@ -2089 +2093 @@
             page->chrome()->focus();
 }
-        
+
 void FrameLoader::loadFrameRequestWithFormAndValues(const FrameLoadRequest& request, bool lockHistory, Event* event,
     HTMLFormElement* submitForm, const HashMap<String, String>& formValues)
 {
     RefPtr<FormState> formState;
-    if (submitForm && !formValues.isEmpty())
+    if (submitForm)
         formState = FormState::create(submitForm, formValues, m_frame);
-    
+
     loadFrameRequestWithFormState(request, lockHistory, event, formState.release());        
 }

trunk/WebCore/loader/FrameLoader.h

@@ -387 +387 @@
 
         void clearRecordedFormValues();
-        void recordFormValue(const String& name, const String& value, PassRefPtr<HTMLFormElement>);
+        void setFormAboutToBeSubmitted(PassRefPtr<HTMLFormElement> element);
+        void recordFormValue(const String& name, const String& value);
 
         bool isComplete() const;