Bug 92358

Created attachment 154611 [details] Patch

Comment on attachment 154611 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154611&action=review > Source/WebKit2/UIProcess/API/efl/ewk_form_submission_request.h:56 > + * When the reference count reaches 0, the intent is freed. Intent??? Also shouldnt you say thta it will be submitted if not already. > Source/WebKit2/UIProcess/API/efl/ewk_form_submission_request.h:99 > +EAPI Eina_Bool ewk_form_submission_request_submit(Ewk_Form_Submission_Request *request); hmm here you could read it as a "request submit" instead of submit a submission_request. I dont have any better ideas though > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:948 > + * Emits signal: "form,request,new" with pointer to Ewk_Form_Submission_Request. form,request,submit ? isn't that not more obvious? or form,submission,new? > Source/WebKit2/UIProcess/API/efl/ewk_view.h:33 > + * It is possible to handle the form submission request asynchronously, by simply calling it is possible to intercept...? > Source/WebKit2/UIProcess/API/efl/ewk_view.h:37 > + * ewk_form_submission_request_submit() will be called. called automatically?

Comment on attachment 154611 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154611&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:948 >> + * Emits signal: "form,request,new" with pointer to Ewk_Form_Submission_Request. > > form,request,submit ? isn't that not more obvious? or form,submission,new? Hmm, how about "form,submission,request" ? >> Source/WebKit2/UIProcess/API/efl/ewk_view.h:33 >> + * It is possible to handle the form submission request asynchronously, by simply calling > > it is possible to intercept...? No, there is no such method in the C API, we can only continue the submission, not cancel it.

Created attachment 154617 [details] Patch Take Kenneth feedback into consideration.

Comment on attachment 154617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154617&action=review > Source/WebKit2/UIProcess/API/efl/ewk_form_submission_request.cpp:49 > + bool handledRequest; bool wasHandled ? or maybe just bool submitted; handledRequest sounds a bit weird, it is more like did handle request or request was handled > Source/WebKit2/UIProcess/API/efl/ewk_form_submission_request.cpp:64 > + // Make sure the request is always handled before destroying. > + if (!handledRequest) > + WKFormSubmissionListenerContinue(wkListener.get()); As this is more like a "already done, break" if sentence I would write it the other way around if (handled) return ... This is generally a good way because it allows for less errors when making refactorings > Source/WebKit2/UIProcess/API/efl/ewk_view.h:30 > + * The Ewk_Form_Submission_Request passed contains information about the text fields of the form. This What about other forms such as checkboxes, etc? > Source/WebKit2/UIProcess/API/efl/ewk_view_form_client.cpp:46 > + memset(&formClient, 0, sizeof(WKPageFormClient)); Any reason to null it if you are filling out all the values? Just to be safe for future extensions?

Comment on attachment 154617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=154617&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_form_submission_request.cpp:49 >> + bool handledRequest; > > bool wasHandled ? or maybe just bool submitted; > > handledRequest sounds a bit weird, it is more like did handle request or request was handled Ok. >> Source/WebKit2/UIProcess/API/efl/ewk_form_submission_request.cpp:64 >> + WKFormSubmissionListenerContinue(wkListener.get()); > > As this is more like a "already done, break" if sentence I would write it the other way around > > if (handled) > return > > ... > > This is generally a good way because it allows for less errors when making refactorings Ok. >> Source/WebKit2/UIProcess/API/efl/ewk_view.h:30 >> + * The Ewk_Form_Submission_Request passed contains information about the text fields of the form. This > > What about other forms such as checkboxes, etc? No, only test fields values are passed. >> Source/WebKit2/UIProcess/API/efl/ewk_view_form_client.cpp:46 >> + memset(&formClient, 0, sizeof(WKPageFormClient)); > > Any reason to null it if you are filling out all the values? Just to be safe for future extensions? Yes, for possible future extensions: this is safer.

Comment on attachment 154617 [details] Patch Clearing flags on attachment: 154617 Committed r123742: <http://trac.webkit.org/changeset/123742>

All reviewed patches have been landed. Closing bug.