Closed Bug 118828 Opened 23 years ago Closed 13 years ago

Editing plugin header files is a recipe for disaster

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED
Future

People

(Reporter: bnesse, Assigned: jaas)

References

Details

(Whiteboard: [PL2:P4])

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.
cc:ing Adam Lock for npapi.h used by embedding
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.
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.
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.
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.
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
Depends on: 88998
Priority: -- → P3
Whiteboard: [PL2:P4]
Target Milestone: --- → mozilla1.2alpha
Priority: P3 → P4
Target Milestone: mozilla1.2alpha → mozilla1.2beta
Target Milestone: mozilla1.2beta → mozilla1.3alpha
moving to future
Target Milestone: mozilla1.3alpha → Future
QA Contact: shrir → plugins
I fixed this a while ago. We don't have duplicate npapi.h files or nsplugindefs.h any more.
Assignee: serhunt → joshmoz
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.