Closed Bug 544698 Opened 10 years ago Closed 10 years ago

Clean up form submission code


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

Not set





(Reporter: sicking, Assigned: sicking)




(4 files)

I'd imagine that there is a lot of cleanup that can happen here. However I'm worried about making behavioral changes at this point.

Attaching two patches. Some move some of the logic out of the form submission code (specifically the stuff in nsFormSubmission::ProcessValue), and moves it into the relevant elements instead. Also contains some decomtamination.

Second patch just moves code around to avoid having to forward declare functions, and removes a pile of useless/out-of-date comments.
By moving out value processing out to the elements the submission code no longer has to deal with "form processors", and also don't need to get a handle to the element in AddNameValuePair/AddNameFilePair.
Assignee: nobody → jonas
Attachment #425667 - Flags: review?(jst)
No code changes, just move functions around to avoid having to forward declare. Add and remove comments to make code more readable.
Attachment #425668 - Flags: review?(jst)
Is this for all operating systems?
Yes, this is all cross platform code.
OS: Mac OS X → All
Hardware: x86 → All
There is only one place in the code where there are two things pointing to the same nsFormSubmission, during nsHTMLFormElement::FlushPendingSubmission.

However as far as I can see there is no need to let mPendingSubmission point to the submission we're submitting, so just hold a local variable instead.
Attachment #427722 - Flags: review?(jst)
So this (final) patch contains the following cleanups:

Move the creation of the encoder to the nsFormSubmission ctor. This way we don't have to create the encoder outside of the submission and then pass both the charset and the charset encoder through all subclass ctors. Instead we just pass the charset and then the encoder is constructed in the ctor.

Move the call to linkHandler->OnLinkClickSync from the submission code to the form element code.
Attachment #427724 - Flags: review?(jst)
Comment on attachment 425667 [details] [diff] [review]
Part1: Move value "processing" from submission-code to elements. Kill dead code. Decomtaminate

Looks good! Caught one typo...

- In nsFormSubmission::Release():

+    mRefCnt = 1; // stabalize


Attachment #425667 - Flags: review?(jst) → review+
Attachment #425668 - Flags: review?(jst) → review+
Attachment #427722 - Flags: review?(jst) → review+
Attachment #427724 - Flags: review?(jst) → review+
Depends on: 567723
Depends on: 607217
No longer depends on: 607217
Depends on: 644138
Depends on: 619285
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.