Summary: | Add ability to send UserData to willSubmitForm | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||
Component: | WebKit2 | Assignee: | Sam Weinig <sam> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | eric, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Sam Weinig
2010-09-03 18:37:01 PDT
Created attachment 66581 [details]
Patch
Attachment 66581 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:150: Extra space between WKBundlePageWillSubmitFormCallback and willSubmitForm [whitespace/declaration] [3]
WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.h:35: Code inside a namespace should not be indented. [whitespace/indent] [4]
WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:31: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 3 in 12 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 66581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66581&action=prettypatch > WebKit2/UIProcess/WebPageProxy.h:205 > + void willSubmitForm(WebFrameProxy* frame, WebFrameProxy* frameSource, Vector<std::pair<WTF::String, WTF::String> >& textFieldValues, APIObject*, uint64_t listenerID); I think it would be helpful to name the argument here. > WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.h:45 > +class APIObject; > class WebFrame; > class WebPage; > +class ImmutableDictionary; Please sort. r=me Comment on attachment 66581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66581&action=prettypatch Mostly just curious what this code looked like. Very pretty. Some (trivial) comments below. > WebKit2/UIProcess/WebPageProxy.cpp:581 > - willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, listenerID); > + willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, 0/*userData*/, listenerID); Slightly more beautiful would be a noUserData constant that was just a null pointer. > WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:513 > + RefPtr<FormState> formState = prpFormState; I've never really like this pattern, but I don't see a way around it. > WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:520 > + RefPtr<APIObject> userData; > + webPage->injectedBundleFormClient().willSubmitForm(webPage, form, m_frame, sourceFrame, values, userData); I'm confused. Don't we need to actually allocate a userData object here? Does willSubmitForm take a RefPtr by reference as an out param? (In reply to comment #4) > (From update of attachment 66581 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=66581&action=prettypatch > > Mostly just curious what this code looked like. Very pretty. Some (trivial) comments below. > > > WebKit2/UIProcess/WebPageProxy.cpp:581 > > - willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, listenerID); > > + willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, 0/*userData*/, listenerID); > Slightly more beautiful would be a noUserData constant that was just a null pointer. Ok. > > > WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:520 > > + RefPtr<APIObject> userData; > > + webPage->injectedBundleFormClient().willSubmitForm(webPage, form, m_frame, sourceFrame, values, userData); > I'm confused. Don't we need to actually allocate a userData object here? Does willSubmitForm take a RefPtr by reference as an out param? Yes it does and very intentionally. Thanks for the review. Attachment 66581 [details] was posted by a committer and has review+, assigning to Sam Weinig for commit.
Fixed in http://trac.webkit.org/changeset/66789. |