Bug 38397 – Add "preSubmitForm" hook to FrameLoader

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Jens Alfke]

In some cases* the browser may need to examine the contents of a form immediately before it's submitted — in particular, before any of the page's onSubmit handlers have had a chance to run. WebKit does not currently have any hook for this, since the existing "willSubmitForm" delegate method in the API does not get called until after the form submission has actually begun.

The enclosed patch remedies this by adding a "dispatchPreSubmitForm" method to the WebCore::FrameLoaderClient interface. (It's not abstract: it defaults to doing nothing. I did this because it won't always be needed, and adding an abstract method to FrameLoaderClient requires modifying half a dozen subclasses for at least four different OSs, to avoid breaking the build.)

I plumbed this through Chromium's WebKit API, because I need this callback for some work I'm doing on Chrome. I didn't touch the APIs for other platforms, but it would of course be easy to add if anyone else needs this functionality.

[* The case in question is <http://code.google.com/p/chromium/issues/detail?id=28910>. Some websites modify the login form, clearing the password field and putting a hashed password into a hidden field, in the onSubmit handler. This breaks Chrome's (and Safari's) password autofill, because by the time it registers that the form was submitted, it's too late to capture the password. The autofill code needs a chance to look at the form before the onSubmit handler runs.]

[Jens Alfke]

Created attachment 54819 [details]
patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 54819 [details]
patch

WebCore/html/HTMLFormElement.cpp:294
 +      frame->loader()->dispatchPreSubmitForm(this);
I would just do frame->loader()->client()->dispatch* here instead
of adding the intermediate FrameLoader method.

WebCore/loader/FrameLoaderClient.h:149
 +          virtual void dispatchPreSubmitForm(HTMLFormElement*) { };
How about naming this dispatchWillSendSubmitEvent?  I think that name is more accurate.

WebKit/chromium/public/WebFrameClient.h:143
 +      virtual void preSubmitForm(WebFrame*, const WebFormElement&) { }
willSendSubmitEvent

[Jens Alfke]

Created attachment 54826 [details]
patch 2

Sounds good. Here's an updated patch.

[Darin Fisher (:fishd, Google)]

Comment on attachment 54826 [details]
patch 2

WebCore/loader/FrameLoaderClient.h:149
> +          virtual void dispatchWillSendSubmitEvent(HTMLFormElement*) { };
no trailing suffix.  actually, this should be pure, and the numerous
implementations of this interface should be updated to have an empty
method.  that way if the method name is changed, it will be easy to
tell if it impacts any ports.

we don't use that rule in WebFrameClient.h since we have different
constraints there:  we are optimizing there for avoiding painful
two-sided commits.

WebKit/chromium/src/FrameLoaderClientImpl.cpp:955
> +  void FrameLoaderClientImpl::dispatchWillSendSubmitEvent(WebCore::HTMLFormElement* form)
no need for WebCore:: prefix in this file.  there is
a 'using namespace WebCore' at the top of the file.

WebKit/chromium/public/WebFrameClient.h:141
> +      // A form submission has been requested, but the page's onSubmit handlers
nit: onSubmit -> submit event

[Jens Alfke]

Created attachment 54836 [details]
patch 3

OK, made the new method abstract. That unfortunately required touching about 8 more files (all the places it's implemented across all platforms.)

[Darin Fisher (:fishd, Google)]

Is the patch missing a change for the win port?

[Alexey Proskuryakov]

> Some websites
> modify the login form, clearing the password field and putting a hashed
> password into a hidden field, in the onSubmit handler.

Is it known why those sites are doing that? Is the purpose to prevent autofill?

[Darin Fisher (:fishd, Google)]

(In reply to comment #7)
> > Some websites
> > modify the login form, clearing the password field and putting a hashed
> > password into a hidden field, in the onSubmit handler.
> 
> Is it known why those sites are doing that? Is the purpose to prevent autofill?

I don't think we know for sure, but it seems that they would just use autocomplete=off if they cared about defeating password managers.

[Jens Alfke]

> Is it known why those sites are doing that? Is the purpose to prevent autofill?

No, the purpose is to avoid sending the password in the clear, without having to implement SSL on the server. I think LiveJournal does it because SSL used to be too expensive, and SimpleMachines does it because they want their forum software to be able to run in a typical hosted web setup.

[Jens Alfke]

Created attachment 54933 [details]
patch 4

Added the new method to Windows WebFrame class.

[WebKit Commit Bot]

Comment on attachment 54933 [details]
patch 4

Clearing flags on attachment: 54933

Committed r58686: <http://trac.webkit.org/changeset/58686>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.