Last Comment Bug 118828 - Editing plugin header files is a recipe for disaster
: Editing plugin header files is a recipe for disaster
Status: RESOLVED FIXED
[PL2:P4]
:
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: All All
: P4 normal with 1 vote (vote)
: Future
Assigned To: Josh Aas
:
:
Mentors:
Depends on: 88998
Blocks:
  Show dependency treegraph
 
Reported: 2002-01-08 13:22 PST by Brian Nesse (gone)
Modified: 2012-01-26 12:47 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Brian Nesse (gone) 2002-01-08 13:22:02 PST
While working on adding support for scrolling notification for plug-ins I have
come across a few things in the plugin header files that concern me greatly.

First of all, there are 2 copies of the file npapi.h in the tree.
/embedding/browser/activex/src/pluginhostctrl/pluginsdk_include/npapi.h
and
/include/npapi.h

Second, in the directory /modules/plugin/base/public there are 2 files,
nsPluginDefs.idl and nsplugindefs.h, which appear to be used somewhat
interchangeably.
http://lxr.mozilla.org/seamonkey/search?string=nsPluginDefs.idl
http://lxr.mozilla.org/seamonkey/search?string=nsplugindefs.h
The problem with this is that while the files are essentially similar, there are
a number of definitions which do not exist in both files.

The combination of these 4 files (assuming there are not more like them) all but
guarantees that mistakes will be made when working in the plug-in code.

We need to find a way to eliminate all of these multiple files and definitions,
perhaps via header restructuring or the creative use of #includes.
Comment 1 Peter Lubczynski 2002-01-08 17:43:00 PST
cc:ing Adam Lock for npapi.h used by embedding
Comment 2 Adam Lock 2002-01-09 04:43:28 PST
I originally checked in npapi.h there so that people could build my plugin with
out downloading the entire plugin sdk. It is used nowhere else and the plugin is
not part of the Mozilla build.

I will review if it is still necessary.
Comment 3 Brian Nesse (gone) 2002-01-09 08:58:10 PST
As av pointed out via email, the copy of npapi.h in the embedding directory is
also out of date. Even if it is still necessary we need to find a way to get
down to one copy of npapi.h.
Comment 4 Adam Lock 2002-01-09 09:05:41 PST
The npapi.h may be out of date for Mozilla, but the plugin host control is not
intended to run in Mozilla! It is an ActiveX control designed to allow plugins
to run in IE and other ActiveX control containers. For that purpose, the plugin
host control implements the NPN API as found in 4.x and uses the header files to
define the various structures and headers it needs to do this.

I would be happy to rename them to avoid confusion, but I would be wary of using
the new headers in Mozilla unless I could be assured that they will remain
backwards compatible with the 4.x plugin SDK headers.
Comment 5 Brian Nesse (gone) 2002-01-09 09:37:25 PST
As far as I know, the headers *must* retain backwards compatibility to 4.x.
Something you may not be aware of is that the version of npapi.h you are using
is not from the 4.x SDK, it is an early version of the one in the Mozilla tree.
There have been some bug fixes along the way.

For good or bad, an LXR search tends to imply the need to change both files if a
change is made. If these files don't need to be kept in lock step
synchronization we should try to keep it from appearing that they do. One way to
do this would be to actually use the 4.x SDK file instead... another would be to
do as you suggest and rename the file. Either way, I believe the current
scenario is bad.
Comment 6 rubydoo123 2002-07-10 11:49:38 PDT
The plug-ins triage team (av, beppe, peterl, serge and shrir) have reviewed this
issue and have made the following determination:

This is dependent upon the work being done in the general clean-up work, adding
dependency
Comment 7 rubydoo123 2002-12-06 15:10:46 PST
moving to future
Comment 8 Josh Aas 2012-01-26 12:47:17 PST
I fixed this a while ago. We don't have duplicate npapi.h files or nsplugindefs.h any more.

Note You need to log in before you can comment on or make changes to this bug.