Let plugins participate in form submission

• Component: Plug-ins
• Status: RESOLVED FIXED

[Deneb Meketa]

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.

[Deneb Meketa]

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.

[Mark Rowe (bdash)]

<rdar://problem/5061202>

[Alexey Proskuryakov]

*** Bug 11804 has been marked as a duplicate of this bug. ***

[Vlad Alexander]

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.

[David Kilzer (:ddkilzer)]

(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)?

[Vlad Alexander]

(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.

[Vlad Alexander]

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.

[David Kilzer (:ddkilzer)]

Equivalent Mozilla/Firefox bug:  https://bugzilla.mozilla.org/show_bug.cgi?id=188938

[Kent Tamura]

*** Bug 50662 has been marked as a duplicate of this bug. ***

[Kent Tamura]

Now we have HTMLObjectElement::appendFormData().
How to implement this?  How to call NPP_GetValue(NPPVformValue, ...) here?

[John Abd-El-Malek]

(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.

[Kenichi Ishibashi]

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,

[Alexey Proskuryakov]

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.

[Kenichi Ishibashi]

Created attachment 99919 [details]
Patch V0

[Kenichi Ishibashi]

(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?

[Vlad Alexander]

>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

[Kenichi Ishibashi]

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

[Anders Carlsson]

I like the overall direction of this patch, but I'd use the name getFormValue instead of tryToGetFormValue.

[Kent Tamura]

(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.

[Kenichi Ishibashi]

Created attachment 100075 [details]
Patch V1

[Kenichi Ishibashi]

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.

[Kenichi Ishibashi]

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.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Anders Carlsson]

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.

[Kenichi Ishibashi]

Created attachment 100203 [details]
Patch V2

[Kenichi Ishibashi]

(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.

[Anders Carlsson]

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.

[Kenichi Ishibashi]

Created attachment 100241 [details]
Patch V3

[Kenichi Ishibashi]

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.

[Kenichi Ishibashi]

Committed r90707: <http://trac.webkit.org/changeset/90707>