544698 - Clean up form submission code

Status: RESOLVED FIXED

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

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: 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.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: 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.

Comment 3

[Ria Klaassen (not reading all bugmail)]

Is this for all operating systems?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yes, this is all cross platform code.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: 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.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: 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.

Comment 7

[Johnny Stenback (: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

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in

http://hg.mozilla.org/mozilla-central/rev/38c07ace8a84
http://hg.mozilla.org/mozilla-central/rev/9b590d74d8a6
http://hg.mozilla.org/mozilla-central/rev/910467d64a1a
http://hg.mozilla.org/mozilla-central/rev/a3805f7d1cca

Thanks for the reviews!