Chromium now can retrieve form value from NPAPI plugins.
Created attachment 100630 [details] Patch V0
Comment on attachment 100630 [details] Patch V0 Attachment 100630 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9022427
Created attachment 100635 [details] Patch V1 (attemps to fix build failure)
Comment on attachment 100635 [details] Patch V1 (attemps to fix build failure) Attachment 100635 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9018468 New failing tests: plugins/form-value.html
Created attachment 100637 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
(In reply to comment #4) > (From update of attachment 100635 [details]) > Attachment 100635 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/9018468 > > New failing tests: > plugins/form-value.html You need to roll chromium_rev in WebKit/chromium/DEPS to at least 92315.
Cc fishd for Chromium WebKit API change.
Comment on attachment 100635 [details] Patch V1 (attemps to fix build failure) View in context: https://bugs.webkit.org/attachment.cgi?id=100635&action=review > Source/WebKit/chromium/public/WebPlugin.h:60 > + virtual bool getFormValue(WebString*) { return false; } I think this method is not very self-explanatory. What is a form value in the context of a plugin? When I think of HTML forms, I expect there to be a key name associated with a value, yet this function does not take a key name parameter. Other question: Do you really need the bool return? Can you perhaps use WebString::isNull() to determine if the plugin has no form value?
Comment on attachment 100635 [details] Patch V1 (attemps to fix build failure) View in context: https://bugs.webkit.org/attachment.cgi?id=100635&action=review Darin, Thank you for taking a look for this. >> Source/WebKit/chromium/public/WebPlugin.h:60 >> + virtual bool getFormValue(WebString*) { return false; } > > I think this method is not very self-explanatory. What is a form value in the context of a plugin? When I think of HTML forms, I expect there to be a key name associated with a value, yet this function does not take a key name parameter. > > Other question: Do you really need the bool return? Can you perhaps use WebString::isNull() to determine if the plugin has no form value? The meaning of the form value of a plugin is specified in the spec (http://www.w3.org/TR/html5/association-of-controls-and-forms.html#constructing-form-data-set) and the key name to be associated with is the value of the 'name' attribute of the corresponding object element. This is why this function does not take a key name parameter. I didn't come up with an appropriate function name other than it.. The spec says "try to obtain a form submission value from the plugin, and if that is successful, append an entry to the form data set with name as the name". By returning boolean value, I just want to recognize whether obtaining form value is failed or succeeded but the value is empty. The current implementation doesn't append the entry for the plugin when the function returns false, while it appends the entry even if the value is empty when the function returns true.
Created attachment 100753 [details] Patch V2
I think the patch is reasonable. fishd, do you have additional comments?
Created attachment 118937 [details] Patch V3
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
(In reply to comment #12) > Created an attachment (id=118937) [details] > Patch V3 Revised the patch. It seems that I also need to add the feature to PPAPI plugins so removed the change in test_expectations.txt for now.
Comment on attachment 118937 [details] Patch V3 View in context: https://bugs.webkit.org/attachment.cgi?id=118937&action=review > Source/WebKit/chromium/public/WebPlugin.h:60 > + // Returns true if the form submission value is successfully obtained Thanks for adding this comment! It helps a lot. nit: please add a new line above this comment since getFormValue and scriptableObject really aren't all that related. > Source/WebKit/chromium/public/WebPlugin.h:63 > + virtual bool getFormValue(WebString*) { return false; } how about passing WebString& here instead? the WebKit API tries to follow WebKit coding conventions, which call for a reference here instead of a pointer.
Comment on attachment 118937 [details] Patch V3 View in context: https://bugs.webkit.org/attachment.cgi?id=118937&action=review Hi Darin, Thank you for review! >> Source/WebKit/chromium/public/WebPlugin.h:60 >> + // Returns true if the form submission value is successfully obtained > > Thanks for adding this comment! It helps a lot. > > nit: please add a new line above this comment since getFormValue and scriptableObject > really aren't all that related. Will do. >> Source/WebKit/chromium/public/WebPlugin.h:63 >> + virtual bool getFormValue(WebString*) { return false; } > > how about passing WebString& here instead? the WebKit API tries to follow WebKit > coding conventions, which call for a reference here instead of a pointer. I already added Chromium side implementation using a pointer: http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/webkit/plugins/npapi/webplugin_impl.h&exact_package=chromium&q=webkit/plugins/npapi/webplugin_impl.h&type=cs&l=72 I'll change Chromium side and then revise the patch.
Created attachment 119130 [details] Patch V4
(In reply to comment #17) > Created an attachment (id=119130) [details] > Patch V4 Changes in Chromium side is landed. http://src.chromium.org/viewvc/chrome?view=rev&revision=114338
Comment on attachment 119130 [details] Patch V4 Clearing flags on attachment: 119130 Committed r102873: <http://trac.webkit.org/changeset/102873>
All reviewed patches have been landed. Closing bug.
Reverted r102873 for reason: Break the build Committed r102881: <http://trac.webkit.org/changeset/102881>
Created attachment 119723 [details] Patch for landing
Comment on attachment 119723 [details] Patch for landing Clearing flags on attachment: 119723 Committed r103147: <http://trac.webkit.org/changeset/103147>