[Webkit-unassigned] [Bug 39430] Refactor form submission code in HTMLFormElement to remove hairballs and add logical clarity.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39430





--- Comment #11 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2010-06-12 18:52:55 PST ---
Thanks for the review!

(In reply to comment #10)
> (From update of attachment 57594 [details])
> > +            // FIXME: This may fire a DOM Mutation Event. Do we really want this here?
> 
> I don’t see any reason to capitalize “Mutation Event”. Also, I’m not sure about the use of the word “may” here. It either will fire the event or won‘t. Maybe the use of “may” is related to the fact that most likely no one is listening so we will optimize out the work.
> 
> > +            setEnctype("application/x-www-form-urlencoded");
> 
> It’s unfortunate that this function has to have a side effect at all. We should explore whether the form element’s enctype actually needs to be changed. I’m guessing that could be incorrect behavior. Sure, we want this encoding type to be used, but doing that by mutating the DOM seems wrong.
> 
> Thus the comment seems to miss a point that is even more pertinent than the the mutation event. Do we want to mutate the form element at all in this case, or is there a better way to handle it? Maybe the form data builder or some other transient object could hold the encoding type.

Exactly -- I already investigated this and fixed just the way you're suggesting in the patch on bug 40184.

> 
> >      return result;
> 
> This really should be returning result.release().

Good point! Will fix on landing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.