Bug 13061 - Let plugins participate in form submission
Summary: Let plugins participate in form submission
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 523.x (Safari 3)
Hardware: All OS X 10.4
: P3 Enhancement
Assignee: Kenichi Ishibashi
URL: http://bugzilla.mozilla.org/show_bug....
Keywords: InRadar
: 11804 50662 (view as bug list)
Depends on: 48821
Blocks: 39021
  Show dependency treegraph
 
Reported: 2007-03-13 16:25 PDT by Deneb Meketa
Modified: 2011-07-10 19:29 PDT (History)
11 users (show)

See Also:


Attachments
Patch V0 (8.84 KB, patch)
2011-07-06 20:08 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Patch V1 (18.06 KB, patch)
2011-07-07 22:44 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.78 MB, application/zip)
2011-07-07 23:02 PDT, WebKit Review Bot
no flags Details
Patch V2 (24.87 KB, patch)
2011-07-08 22:02 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Patch V3 (24.46 KB, patch)
2011-07-10 18:26 PDT, Kenichi Ishibashi
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Deneb Meketa 2007-03-13 16:25:15 PDT
See Mozilla bug 188938 listed in URL field.  In keeping with HTML4, Mozilla has implemented a way for plugins to provide a form-value string when they are nested within a <form> tag.  This is just a simple NPAPI addition, NPP_GetValue(NPPVformValue), returning a string that the browser then provides as the value associated with the plugin's "name" property from its <object> or <embed> tag.

See also WebKit bug 13029, in which a patch is proposed that will bring npapi.h past the point where it already includes the necessary interface definition.  If the patch for 13029 is landed, it will contain only the interface definition, and not any actual code to take advantage of the new interface.
Comment 1 Deneb Meketa 2007-03-13 16:37:01 PDT
And by the way, this is probably obvious, but the Mozilla solution is for Mozilla plugins.  Presumably WebKit would want to implement this for both WebKit plugins and Mozilla plugins.
Comment 2 Mark Rowe (bdash) 2007-03-13 16:54:49 PDT
<rdar://problem/5061202>
Comment 3 Alexey Proskuryakov 2007-03-21 13:39:32 PDT
*** Bug 11804 has been marked as a duplicate of this bug. ***
Comment 4 Vlad Alexander 2007-03-21 14:16:09 PDT
IE, Firefox and Opera support this feature and it order to provide a cross browser plug-in integration technique, it would be wonderful if Safari supported this as well.

Currently, in the absence of this feature, our users have to add a hidden field to the Web page and a JavaScript function to copy data from the "object" element to the hidden field. Then call this JavaScript function in the "onsumit" attribute of the "form" element. The JavaScript approach has 3 problems:

1. It's complex especially when you have multiple plug-ins on the same page.

2. It's not accessible. We are trying to make our plug-in ATAG- (W3C Authoring Tool Accessibility Guidelines) and ISO-compliant, which means it has to work in the browser when JavaScript is disabled.

3. Our plug-in is used in content management systems (CMS) and many CMS do not permit modification of the "onsubmit" attribute in the "form" element.

A dev build of our plug-in that supports the "name" attribute on the "object" element and a test-case is available here:
http://misc.xstandard.com/apple/javascriptfree.asp

Also, this bug was file in Apple's bug tracking system in January 2005.
Comment 5 David Kilzer (:ddkilzer) 2007-03-21 22:55:54 PDT
(In reply to comment #4)
> Also, this bug was file in Apple's bug tracking system in January 2005.

Vlad, what was the original Radar bug number that you filed for this (if you still have it)?

Comment 6 Vlad Alexander 2007-03-22 03:18:47 PDT
(In reply to comment #5)
> Vlad, what was the original Radar bug number that you filed for this (if you
> still have it)?

Thanks for looking into this bug David. It's:

#3964087 - <object> should submit data to server when "name" attribute is present (new plug-in API)

Over the last 2 years we have also contacted our developer rep at Apple several times to convey the importance of this particular bug to us and our customers.
Comment 7 Vlad Alexander 2007-09-28 07:54:35 PDT
Just following up on the status of this bug. Our commercial browser plug-in (http://xstandard.com) has to use JavaScript/hidden field hack to overcome this bug. So it is very important for us to have this bug fixed. Thanks in advance.
Comment 8 David Kilzer (:ddkilzer) 2007-09-28 09:42:22 PDT
Equivalent Mozilla/Firefox bug:  https://bugzilla.mozilla.org/show_bug.cgi?id=188938
Comment 9 Kent Tamura 2010-12-08 15:27:48 PST
*** Bug 50662 has been marked as a duplicate of this bug. ***
Comment 10 Kent Tamura 2010-12-09 22:28:04 PST
Now we have HTMLObjectElement::appendFormData().
How to implement this?  How to call NPP_GetValue(NPPVformValue, ...) here?
Comment 11 John Abd-El-Malek 2010-12-10 10:26:04 PST
(In reply to comment #10)
> Now we have HTMLObjectElement::appendFormData().
> How to implement this?  How to call NPP_GetValue(NPPVformValue, ...) here?

hey Kent, I'm not that familiar with the WebCore side of plugins.  Chromium and WebKit have different host implementations of NPAPI.  I would set a breakpoint in Chrome's NPP_GetValue and see how it got there from WebCore.
Comment 12 Kenichi Ishibashi 2011-01-11 02:31:23 PST
Hi,

I investigated how we can get the value from plugins on Safari mac port.

On MacOS 10.6, plugins are handled by the separate process (WebKitPluginAgent) and we cannot call NPP_GetValue() directory. I found the MIG specification file for the IPC (WebKitPluginHost.defs) but it seems that there is no interface to call plugin functions such as NPP_GetValue. AFAIK, the WebKitPluginAgent is not a part of WebKit so we cannot add this feature on MacOS 10.6 as long as the WebKitPluginAgent doesn't provide a way to call NPP_GetValue().

On MacOS 10.5 (or earlier, I'm not sure because I don't have any machine earlier than 10.5), plugins are handled by the WebNetscapePluginView class so I think we can implement this feature by adding a method that calls NPP_GetValue in the class. I'm going to implement this before long.

Regards,
Comment 13 Alexey Proskuryakov 2011-01-11 09:03:11 PST
Yes, NPP_GetValue is not currently exposed by the plug-in host. While writing the code for single process case only could be a useful learning experiment, I don't think that there is any good reason to land it.
Comment 14 Kenichi Ishibashi 2011-07-06 20:08:17 PDT
Created attachment 99919 [details]
Patch V0
Comment 15 Kenichi Ishibashi 2011-07-06 20:11:37 PDT
(In reply to comment #14)
> Created an attachment (id=99919) [details]
> Patch V0

This patch introduces an interface method that tries to get form value from the plugin to PluginViewBase.  This patch also contains an implementation for non-hosted plugins on mac (this should will work on Leopard).  For hosted plugins (on Snow Leopard or later), we will be able to implement this once we have an IPC method that allows us to call NPP_GetValue(NPPVformValue, ...).

Tests are missing in this patch for now (That's why I don't r? for the patch).  Testing this requires a plugin, but plugins depend on platform so I cannot use layout tests.  I found tests for plugins in Source/WebCore/manual-tests.  How does it sound that writing a plugin and put a test in Source/WebCore/manual-tests?
Comment 16 Vlad Alexander 2011-07-07 06:17:22 PDT
>Testing this requires a plugin
You can use XStandard WYSIWYG editor plug-in:

For Windows:
http://xstandard.com/download/x-lite.exe

For OS X 10.6:
http://belus.com/download/x-lite.dmg

Test page:
http://misc.xstandard.com/apple/javascriptfree.asp
Comment 17 Kenichi Ishibashi 2011-07-07 06:51:18 PDT
Hi Vlad,

Thank you for the information.  I can use these plugins for local testing (actually, I've tested my patch by using thme :), but we should add tests into WebKit repository so that anyone can test.  This means that we need to write a plugin.

(In reply to comment #16)
> >Testing this requires a plugin
> You can use XStandard WYSIWYG editor plug-in:
> 
> For Windows:
> http://xstandard.com/download/x-lite.exe
> 
> For OS X 10.6:
> http://belus.com/download/x-lite.dmg
> 
> Test page:
> http://misc.xstandard.com/apple/javascriptfree.asp
Comment 18 Anders Carlsson 2011-07-07 13:07:49 PDT
I like the overall direction of this patch, but I'd use the name getFormValue instead of tryToGetFormValue.
Comment 19 Kent Tamura 2011-07-07 17:28:45 PDT
(In reply to comment #15)
> Tests are missing in this patch for now (That's why I don't r? for the patch).  Testing this requires a plugin, but plugins depend on platform so I cannot use layout tests.  I found tests for plugins in Source/WebCore/manual-tests.  How does it sound that writing a plugin and put a test in Source/WebCore/manual-tests?

I guess we can add NPP_GetValue handler to Tools/DumpRenderTree/TestNetscapePlugIn/ and use it for layout tests.
Comment 20 Kenichi Ishibashi 2011-07-07 22:44:32 PDT
Created attachment 100075 [details]
Patch V1
Comment 21 Kenichi Ishibashi 2011-07-07 22:45:44 PDT
Hi Anders,

(In reply to comment #18)
> I like the overall direction of this patch, but I'd use the name getFormValue instead of tryToGetFormValue.

Thank you for suggestion.  I've changed the function name to getFormValue.
Comment 22 Kenichi Ishibashi 2011-07-07 22:47:29 PDT
Hi Kent-san,

(In reply to comment #19)
> (In reply to comment #15)
> > Tests are missing in this patch for now (That's why I don't r? for the patch).  Testing this requires a plugin, but plugins depend on platform so I cannot use layout tests.  I found tests for plugins in Source/WebCore/manual-tests.  How does it sound that writing a plugin and put a test in Source/WebCore/manual-tests?
> 
> I guess we can add NPP_GetValue handler to Tools/DumpRenderTree/TestNetscapePlugIn/ and use it for layout tests.

It's what I'm looking for.  I've revised the patch and wrote a test which uses it.  Thank you letting me know that.
Comment 23 WebKit Review Bot 2011-07-07 23:02:46 PDT
Comment on attachment 100075 [details]
Patch V1

Attachment 100075 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9000418

New failing tests:
http/tests/misc/slow-loading-mask.html
http/tests/navigation/javascriptlink-frames.html
plugins/form-value.html
fast/blockflow/japanese-rl-selection.html
svg/transforms/text-with-mask-with-svg-transform.svg
fast/backgrounds/background-leakage.html
fast/box-shadow/scaled-box-shadow.html
fast/backgrounds/repeat/negative-offset-repeat.html
svg/W3C-SVG-1.1/struct-use-01-t.svg
transforms/2d/hindi-rotated.html
svg/repaint/filter-repaint.svg
fast/blockflow/japanese-lr-selection.html
Comment 24 WebKit Review Bot 2011-07-07 23:02:52 PDT
Created attachment 100078 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 25 Anders Carlsson 2011-07-08 09:13:10 PDT
Comment on attachment 100075 [details]
Patch V1

View in context: https://bugs.webkit.org/attachment.cgi?id=100075&action=review

> Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:809
> +    if (variable == NPPVformValue) {
> +        static const char formValue[] = "Plugin form value";
> +        *((void**)value) = browser->memalloc(sizeof(formValue));
> +        if (!*((void**)value))
> +            return NPERR_OUT_OF_MEMORY_ERROR;
> +        strncpy(*((char**)value), formValue, sizeof(formValue));
> +        return NPERR_NO_ERROR;
> +    }

We've been trying to put less code in the main.cpp part of the test plug-in, and instead factor test specific code out into TestNetscapePlugIn/Tests/. It should be straightforward to add a specific test file just for this.
Comment 26 Kenichi Ishibashi 2011-07-08 22:02:09 PDT
Created attachment 100203 [details]
Patch V2
Comment 27 Kenichi Ishibashi 2011-07-08 22:06:07 PDT
(In reply to comment #25)
> We've been trying to put less code in the main.cpp part of the test plug-in, and instead factor test specific code out into TestNetscapePlugIn/Tests/. It should be straightforward to add a specific test file just for this.

Added as TestNetscapePlugin/Tests/FormValue.cpp.  Thanks!

I've also updated LayoutTests/platform/chromium/test_expectations.txt to skip testing plugins/form-value.html.
Comment 28 Anders Carlsson 2011-07-10 09:52:05 PDT
Comment on attachment 100203 [details]
Patch V2

View in context: https://bugs.webkit.org/attachment.cgi?id=100203&action=review

Looks great! So close to be able to land!

> Tools/DumpRenderTree/TestNetScapePlugIn/Tests/FormValue.cpp:31
> +class FormValuePlugin : public PluginTest {

Can we just call this "FormValue"? The Plugin adds nothing extra here. Similarly we'd name the test "form-value"

> Tools/DumpRenderTree/TestNetScapePlugIn/Tests/FormValue.cpp:50
> +    return PluginTest::NPP_GetValue(variable, value);

You should return NPERR_GENERIC_ERROR here instead of the base class.
Comment 29 Kenichi Ishibashi 2011-07-10 18:26:34 PDT
Created attachment 100241 [details]
Patch V3
Comment 30 Kenichi Ishibashi 2011-07-10 18:28:16 PDT
Comment on attachment 100203 [details]
Patch V2

View in context: https://bugs.webkit.org/attachment.cgi?id=100203&action=review

I've addressed your comments. Thank you for review!

>> Tools/DumpRenderTree/TestNetScapePlugIn/Tests/FormValue.cpp:31
>> +class FormValuePlugin : public PluginTest {
> 
> Can we just call this "FormValue"? The Plugin adds nothing extra here. Similarly we'd name the test "form-value"

Done.

>> Tools/DumpRenderTree/TestNetScapePlugIn/Tests/FormValue.cpp:50
>> +    return PluginTest::NPP_GetValue(variable, value);
> 
> You should return NPERR_GENERIC_ERROR here instead of the base class.

Done.
Comment 31 Kenichi Ishibashi 2011-07-10 19:29:24 PDT
Committed r90707: <http://trac.webkit.org/changeset/90707>