We should use the didAssociateFormControls and shouldNotifyOnFormChanges callbacks on ChromeClient. <rdar://problem/13905067>
Created attachment 202124 [details] Patch
Comment on attachment 202124 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202124&action=review > Source/WebKit2/ChangeLog:26 > + At this time, there is no known use for the elements themselves, so don't bother piping > + that through to the API. Hmm. I wonder if checks in injected bundle will be efficient enough without knowing the elements. These calls might be made frequently on certain pages.
(In reply to comment #2) > (From update of attachment 202124 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202124&action=review > > > Source/WebKit2/ChangeLog:26 > > + At this time, there is no known use for the elements themselves, so don't bother piping > > + that through to the API. > > Hmm. I wonder if checks in injected bundle will be efficient enough without knowing the elements. These calls might be made frequently on certain pages. Thanks for the review! I discussed this further with you and others, and decided to add the elements. New patch coming momentarily.
Created attachment 202328 [details] Patch (version 2)
Comment on attachment 202328 [details] Patch (version 2) View in context: https://bugs.webkit.org/attachment.cgi?id=202328&action=review > Source/WebKit2/ChangeLog:20 > + (InjectedBundlePageFormClient): It's best to remove such unhelpful lines from ChangeLogs (it's a bug in the tool that it generates them). > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.cpp:133 > + unsigned size = elements.size(); The type should be size_t. > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.cpp:135 > + Vector<RefPtr<APIObject> > elementHandles; We are now using C++11 ">>" in WebKit2 code (as well as in all Mac code). > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.cpp:138 > + for (unsigned i = 0; i < size; ++i) size_t > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.h:59 > + void didAssociateFormControls(WebPage*, const Vector<RefPtr<WebCore::Element> >&); Same comment about ">>". > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:719 > +void WebChromeClient::didAssociateFormControls(const Vector<RefPtr<WebCore::Element> >& elements) Ditto. > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.h:173 > + virtual void didAssociateFormControls(const Vector<RefPtr<WebCore::Element> >&) OVERRIDE; Ditto. > Tools/TestWebKitAPI/Tests/WebKit2/DidAssociateFormControls_Bundle.cpp:39 > + virtual void didCreatePage(WKBundleRef, WKBundlePageRef); OVERRIDE?
(In reply to comment #5) > (From update of attachment 202328 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202328&action=review > > > Source/WebKit2/ChangeLog:20 > > + (InjectedBundlePageFormClient): > > It's best to remove such unhelpful lines from ChangeLogs (it's a bug in the tool that it generates them). Done. > > > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.cpp:133 > > + unsigned size = elements.size(); > > The type should be size_t. Done. > > > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.cpp:135 > > + Vector<RefPtr<APIObject> > elementHandles; > > We are now using C++11 ">>" in WebKit2 code (as well as in all Mac code). I initially had that, but check-webkit-style complained. I fixed this and Filed https://bugs.webkit.org/show_bug.cgi?id=116474 > > > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.cpp:138 > > + for (unsigned i = 0; i < size; ++i) > > size_t Done. > > > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.h:59 > > + void didAssociateFormControls(WebPage*, const Vector<RefPtr<WebCore::Element> >&); > > Same comment about ">>”. Done. > > > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:719 > > +void WebChromeClient::didAssociateFormControls(const Vector<RefPtr<WebCore::Element> >& elements) > > Ditto. Done. > > > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.h:173 > > + virtual void didAssociateFormControls(const Vector<RefPtr<WebCore::Element> >&) OVERRIDE; > > Ditto. Done. > > > Tools/TestWebKitAPI/Tests/WebKit2/DidAssociateFormControls_Bundle.cpp:39 > > + virtual void didCreatePage(WKBundleRef, WKBundlePageRef); > > OVERRIDE? Done. Thanks for the review!
Comment on attachment 202328 [details] Patch (version 2) Committed with the changes mentioned above in http://trac.webkit.org/changeset/150398