Clean up form submission code

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: sicking, Assigned: sicking)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

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.
Created attachment 425667 [details] [diff] [review]
Part1: Move value "processing" from submission-code to elements. Kill dead code. Decomtaminate

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)
Created attachment 425668 [details] [diff] [review]
Part2: Reorganize nsFormSubmission.cpp

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
Created attachment 427722 [details] [diff] [review]
Part3: Make nsFormSubmissions not be refcounted

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)
Created attachment 427724 [details] [diff] [review]
Part4: Encoder related cleanup

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

s/stabalize/stabilize/.

r=jst
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
No longer depends on: 607217
You need to log in before you can comment on or make changes to this bug.