Potential discrepancies in some variable definitions

RESOLVED FIXED in Future

Status

()

Core
Plug-ins
P4
normal
RESOLVED FIXED
17 years ago
6 years ago

People

(Reporter: av (gone), Assigned: Josh Aas)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PL2:P4])

(Reporter)

Description

17 years ago
We have a whole bunch of variables that are used in GetValue and SetValue calls 
in both old and new plugin APIs some of them correspond to each other but have 
different values in the two worlds. I am filing this bug to trigger a discussion 
on whether it could have a negative impact and if it is not too late to fix if 
necessary.

In the old API we have the following (npapi.h):

typedef enum {
  NPPVpluginNameString = 1,    //1
  NPPVpluginDescriptionString, //2
  NPPVpluginWindowBool,        //3
  NPPVpluginTransparentBool,   //4
  NPPVjavaClass,               //5
  NPPVpluginWindowSize,        //5
  NPPVpluginTimerInterval      //6
} NPPVariable;

typedef enum {
  NPNVxDisplay = 1,          //1
  NPNVxtAppContext,          //2
  NPNVnetscapeWindow,        //3
  NPNVjavascriptEnabledBool, //4
  NPNVasdEnabledBool,        //5
  NPNVisOfflineBool          //6
} NPNVariable;

In the new API we have (nsplugindefs.h):

enum nsPluginVariable {
  nsPluginVariable_NameString                = 1,
  nsPluginVariable_DescriptionString         = 2
};

enum nsPluginManagerVariable {
  nsPluginManagerVariable_XDisplay           = 1,
  nsPluginManagerVariable_XtAppContext       = 2
};

enum nsPluginInstancePeerVariable {
  nsPluginInstancePeerVariable_NetscapeWindow  = 3
//  nsPluginInstancePeerVariable_JavaClass     = 5,
//  nsPluginInstancePeerVariable_TimerInterval = 7
};

enum nsPluginInstanceVariable {
  nsPluginInstanceVariable_WindowlessBool          = 3,
  nsPluginInstanceVariable_TransparentBool         = 4,
  nsPluginInstanceVariable_DoCacheBool             = 5,
  nsPluginInstanceVariable_CallSetWindowAfterDestroyBool = 6
};

Not only have some of them not matching values but to my understanding are 
placed under wrong type. I think more correct arrangement in the new API would 
be:

// the following two types are corresponding NPPVariable
enum nsPluginVariable {
  nsPluginVariable_NameString         = 1,
  nsPluginVariable_DescriptionString  = 2
+//  nsPluginVariable_JavaClass         = 5,
+//  nsPluginVariable_TimerInterval     = 7 not sure, maybe 
                                            nsPluginInstanceVariable?
};

enum nsPluginInstanceVariable {
  nsPluginInstanceVariable_WindowlessBool          = 3,
  nsPluginInstanceVariable_TransparentBool         = 4,
-  nsPluginInstanceVariable_DoCacheBool             = 5,
-  nsPluginInstanceVariable_CallSetWindowAfterDestroyBool = 6
+  nsPluginInstanceVariable_PluginWindowSize        = 6, 
+  nsPluginInstanceVariable_DoCacheBool                   = 8,
+  nsPluginInstanceVariable_CallSetWindowAfterDestroyBool = 9
};

// the following two types are corresponding NPNVariable
enum nsPluginManagerVariable {
  nsPluginManagerVariable_XDisplay           = 1,
  nsPluginManagerVariable_XtAppContext       = 2
  nsPluginManagerVariable_JSBool             = 4,
  nsPluginManagerVariable_asdBool            = 5,
  nsPluginManagerVariable_OfflineBool        = 6
};

enum nsPluginInstancePeerVariable {
  nsPluginInstancePeerVariable_NetscapeWindow  = 3
-//  nsPluginInstancePeerVariable_JavaClass     = 5,
-//  nsPluginInstancePeerVariable_TimerInterval = 7
};

My biggest concern if we are going to fix it is DoCache and 
CallSetWindowAfterDestroy.

Comment 1

17 years ago
Changing the value of nsPluginInstanceVariable_DoCacheBool or 
nsPluginInstanceVariable_CallSetWindowAfterDestroyBool will definitely break 
the Beatnik Player xpcom plugin.  It's explicitly setting the value for those 2 
when my nsIPluginInstance::GetValue() method is called.
Summary: Potential discreapancies in some variable definitions → Potential discrepancies in some variable definitions
(Reporter)

Comment 2

17 years ago
Did we already have those two in NS6 RTM? I just cannot immediately recall...

Comment 3

17 years ago
Yes - these were added Sept 14 2000:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/modules/plugin/public/nsplugindefs.h

Comment 4

17 years ago
av: Sean's got it; the changes were the result of a meeting amongst Netscape and
plugin vendors, to discuss changes in the plugin lifecycle.  One problem that I
know of is that the Sun Java Plug-In had problems with the existing lifecycle,
so we lobbied to change it.  We also lobbied to have a flag that a plug-in could
set, asking whether to cache a plugin instance; the Java Plug-In wanted control
over applet caching, not having the browser control it.
Does that ring a bell?  If not, lemme know, and I'll happily send you the
meeting notes written by Eric Krock.
(Reporter)

Comment 5

17 years ago
No, no, George, I remember all this. It just came up recently that there is no 
uniformity and variable values overlap partially, so some binaries may confuse 
this DoCache thing with GetJavaClass which may probably not be that important 
since new plugins shouldn't worry about java class anyway. Also I noticed that 
some variables belong in wrong places. Which is annoying but not a big issue.

Comment 6

16 years ago
Hm...this explain a lot. If Java is reporting it's cache variable incorrectly 
then that explain why may patch to crashing in ~nsPluginInstanceOwner 
on release works for 4.x plugins and Real Player XPCOM but not Java. I 
will take a closer loook.

Comment 7

16 years ago
peter? any update on this? Any thing I can do to help investigate this?

anthonyd
Target Milestone: --- → Future

Comment 8

16 years ago
I don't know what can be done about this now. I suggest staying clear of these
messed up variables.

Comment 9

15 years ago
leaving in future, we will try to resolve this during general plug-in clean-up
Priority: -- → P5
Whiteboard: [PL2:P4]

Updated

15 years ago
Priority: P5 → P4
QA Contact: shrir → plugins
(Assignee)

Comment 10

6 years ago
I cleaned up npapi.h vs. nsplugindefs.h discrepancies a while ago, basically by eliminating the latter.
Assignee: serhunt → joshmoz
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.