141830 - scriptable plugin documentation refers to outdated sample code

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Plug-ins, defect, P3)

Description

[Shilad Sen]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0rc1) Gecko/20020417
BuildID:    2002041711

Right now linux 4x plugins that use npunix.c with the new scriptable interface
are not scriptable.  This is because npunix.c does not fill in the "getvalue"
member of the NPPluginFuncs in the NP_Initialize() functions.

The following line must be included when the table is filled in (line 463):

pluginFuncs->getvalue   = NewNPP_GetValueProc(Private_GetValue);

To support this function, Private_GetValue must be defined:

NPError
Private_GetValue(NPP instance, NPPVariable variable, void *r_value)
{
    PLUGINDEBUGSTR("GetValue");
    return NPP_GetValue(instance, variable, r_value);
}



Reproducible: Always
Steps to Reproduce:
1.  Implement a 4.x legacy plugin and try to make it scriptable via a scriptable
peer. 
2.  NPP_GetValue is never called on the plugin.

Comment 1

[Peter Lubczynski]

This is only a problem with the sample code in the tree, right?

Comment 2

[Shilad Sen]

I don't think so.  All C "NPP_" style plugins are supposed to compile npunix.c
(as is) and link against it.  The programmer should not have to touch (or
understand) npunix.c.

Comment 3

[rubydoo123]

handing over to serge, and putting into 1.2

Comment 4

[av (gone)]

Shilad, what npunix.c? From the old Plugin SDK?

Comment 5

[Shilad Sen]

It looks to me like the _getvalue table assignment is missing from npunix.c in
both the Netscape download (unix-sdk-3.0b5.tar.Z) and the mozilla source tree
(/modules/plugin/samples/default/unix/npunix.c).

It may be worth revising the docs on the "Scripting Plugins in Mozilla" page to
indicate that the change to npunix.c is necessary.  Otherwise the example (and
the general approach) on the page will not function.

Does a target of 1.2 mean it won't make it in the source tree until Mozilla v.
1.2?

Comment 6

[av (gone)]

Please look at the samples in modules/plugin/tools/sdk. This is the latest and
fullest version of our sample code.

Comment 7

[Peter Lubczynski]

I don't know if the default plugin is a good sample for scripting. Our SDK ones
are probably better.

Comment 8

[rubydoo123]

based on Andrei's comment in #6, marking this as invalid due to using the wrong
samples

Comment 9

[Shilad Sen]

I won't reopen this bug, but the problem arose for me when the sample provided
at http://www.mozilla.org/projects/plugins/scripting-plugins.html wouldn't work.
 Please note that Example 3. from this page CAN NOT work without the "patch" I
submitted.  It would seem to me that either example 3 in "Scripting Plugins in
Mozilla" should be changed, or the patch should be applied.

Shilad

Comment 10

[David Brittain]

I'm reopening this as someone else has described the same problem in a recent
thread in the newsgroup. The documentation needs fixing.

Comment 11

[serge (gone)]

I not quiet sure I understand what is this bug about,
default plugins on any platforms
http://lxr.mozilla.org/mozilla/source/modules/plugin/samples/default
are not scriptable by design.
If there's need of scriptable example, take a look here
http://lxr.mozilla.org/mozilla/source/modules/plugin/tools/sdk/samples/scriptable
-->INVALID.

Comment 12

[Shilad Sen]

Serge,

The reason I ran into this problem (and the reason the reason others on the
plugins mailling list also have) is that the documentation on mozilla.org is wrong.

If you go to the plugin main page, under "how to make a plugin scriptable" there
is a link to

http://www.mozilla.org/projects/plugins/scripting-plugins.html

Note that this approach (in Example 3) indicates using the NPP_ family of
functions, which is, as you say, out of date.  Unfortunately, this is where
developers are pointed and there is no indication that this is out of date.

I would make the request that either this page of documentation is removed, it
is updated, or the patch is applied.  At the moment it misleads developers into
a solution that will not work with the patch I supplied.  I'm happy to write a
new plugin documentation page if that's helpful.

Thanks for your help on this,

Shilad

Comment 13

[serge (gone)]

hmm, I see nothing wrong on
http://www.mozilla.org/projects/plugins/scripting-plugins.html
except it does not point to scriptable examples I mentioned in comment #11
but it does not point to default plugin either.

>...NPP_ family of functions, which is, as you say, out of date
where did I say so?

>I'm happy to write a new plugin documentation page if that's helpful
sure, it'll be helpful, and you are always very welcome to do your changes,
you can reopen this bug and attach the docs here, or open new bug with an
appropriate summary description if you wish.
thanks.

Comment 14

[Shilad Sen]

Serge,

Maybe I'm very confused.  This certainly wouldn't be the first time.  But...

http://www.mozilla.org/projects/plugins/scripting-plugins.html

Examples 1 & 2 seem fine.  Example 3, in my experience, will not work without my
patch.  Note that Examples 1, 2, and 3 are all different parts of the same
"example" plugin.

It seems to me that the plugins SHOULD be using the C++ libraries (such as the
ones used in /modules/plugin/tools/sdk/samples/scriptable/unix) and not the C
NPP_GetValue type functions.  If this is not the case, I repeat my claim that my
patch is necessary.

Let me know if I'm insane,

Shilad

Comment 15

[David Brittain]

I think that the problem here is that the sample code referred to from
http://www.mozilla.org/projects/plugins/scripting-plugins.html has not been
written to work on Unix, so developers looking to make it work take some code
from the default plugin (npunix.c). This code does not build the function
pointer table correctly for getvalue which happens to be something that is
required to make scriptable plugins work.

If the documentation referred to the samples in
modules/plugin/tools/sdk/samples/scriptable instead then there would be no
problem. So I am reopening this bug and changing it to a more appropriate summary.

Comment 16

[serge (gone)]

David, please point to me where that doc refers to outdated sample code?
all samples in that doc are XP, and there is no link to any implementation in
mozilla, which we probably need to add.
If developers want to use default plugin code as a sample to write their own
scriptable plugin they're free to do so, and they'll have to add Shilad's patch
 along with additional code to make such plugin scriptable.
But as I said before default plugin is not an example of scriptable plugin
implementation, it's actually working plugin, which invokes plugin finder
services to find and install plugins w/ unregistered mimetypes.
I see no reason to change its code and create scriptable example from it,
furthermore, native default plugins will be gone after bug 83754 is fixed.

Comment 17

[David Brittain]

Serge, I have to confess to not having tried this myself - perhaps I ought to!
But, I get the impression from two people having come against this independently
that the sample pointed to from the docs does not work on Unix as it stands.

There is a link to sample code in the docs, where it says "The full sample
plugin code can be found in the Mozilla source tree under 
modules/plugin/samples/4x-scriptable". Now if this sample builds and works under
Unix then I agree this bug can be closed. If it doesn't then the document should
be updated to point to the new plugin sample code at
modules/plugin/tools/sdk/samples/scriptable

Comment 18

[serge (gone)]

modules/plugin/samples/4x-scriptable is win & mac only:(
Av, could you update the doc and resolve this bug, please?

Comment 19

[Greg Stacey]

I'm not sure everyone's on the same page on this...  and since I'm one of the 
two people who, independently, found the bug I'll add my two cents.  :)

The reason I followed the examples in the 
http://www.mozilla.org/projects/plugins/scripting-plugins.html page as opposed 
to some of the other samples is because some link (don't recall where) showed 
that page as the fast way to go from Netscape 4.x scriptability (LiveConnect) 
to Netscape 7 / Mozilla scriptability (XPConnect).  The Netscape 4.x interface 
is the same as the one mentioned in this example and, if not for the bug, this 
would have been a very fast fix.  So, while this is not the most correct / up-
to-date way to code a plugin, it is very useful for migrating from 4.x as it 
involves a minimum of changes.

Having said all that, I just found another bug in the npunix.c file in the 
function tables in NP_Initialize.

The following line needs to be added...
 gNetscapeFuncs.posturlnotify = nsTable->posturlnotify;

Otherwise you get an infrequent crash.

Comment 20

[serge (gone)]

>Otherwise you get an infrequent crash
could you provide a test case, please?

Comment 21

[av (gone)]

Is it from the old plugin SDK? I remember some function table fields missing
there, but they are all present in the new sample code.

Serge, do you think that the right solutin here would be fixing the Unix part?
Our current build system is supposed to use the same makefiles for all
platforms, it would be great to know why it fails on Unix.

Comment 22

[Greg Stacey]

Sorry about the infrequent crash comment.  I was having trouble pinpointing it 
at the time.  I now have a reproducible error that crashes each time.  
Presumably it is calling posturlnotify as adding the line I mentioned fixes 
the problem.  Unfortunately I can't find the code for that particular movie 
(the plugin in question is Macromedia Flash) so I can't verify exactly what 
it's doing.

Comment 23

[serge (gone)]

av, this bug is about something missing in
http://lxr.mozilla.org/seamonkey/find?string=npunix.c
which is actually working plugin to handle "*" (default) mimetype,
and if functionality of that plugin does not require to assign into
pluginFuncs->getvalue or
gNetscapeFuncs.posturlnotify
I see no reason to do so.
Once again /modules/plugin/samples/default/unix/npunix.c  is not a current
plugins SDK sample file, but there is the similar or the same file in
ftp://ftp.netscape.com/pub/sdk/plugin/unix/unix-sdk-3.0b5.tar.Z, which is old
4.x SDK. I do not know if we have to update that package or not, but we
definitely have to update the doc
http://www.mozilla.org/projects/plugins/scripting-plugins.html#build
the top bullet of |Related sources|
"The full sample plugin code can be found in the Mozilla source tree under
modules/plugin/samples/4x-scriptable"
should be moved down and modified to "Another sample of scriptable plugin ..."

Comment 24

[av (gone)]

4x-scriptable plugin _is_ a fully functional plugin, I see nothing wrong with
it. There is readme file saying that it is not currently buildable under Unix.
This is our only sample showing two-way JS communication, in which sense it is
'fuller' than any other samples we have.

There is also no mentioning whatsoever of npunix.c in the document nor of the
old Plugin SDK.

Having said that, I don't think the summary line of this bug is correct. The
problem I can imagine some people can experience is that they use their existing
plugin and try to modify it with this new technique and did not have
NPP_GetValue implemented before. Would the following suffice?

   4. Two new cases for the above mentioned new variables should be added to 
- the plugin implementation of NPP_GetValue (see example 3).
+ the plugin implementation of NPP_GetValue (see example 3). If your plugin 
+ did not implement this function before, make sure you assign the function 
+ address to the appropriate field in the plugin function table.

Comment 25

[rubydoo123]

moving to 1.3 alpha

Comment 26

[rubydoo123]

future