Bug 17852 - _NPN_IntFromIdentifier is wrong
Summary: _NPN_IntFromIdentifier is wrong
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-14 13:20 PDT by Eric Seidel (no email)
Modified: 2008-03-17 14:47 PDT (History)
0 users

See Also:


Attachments
First pass at identifier -> int fix and test case (7.06 KB, patch)
2008-03-14 13:20 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Cleanup PluginObject.cpp and fix test methods (24.50 KB, patch)
2008-03-17 13:59 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Fix NPAPI implementation and plugin test (12.17 KB, patch)
2008-03-17 13:59 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
First pass at identifier -> int fix and test case (33.16 KB, patch)
2008-03-17 14:08 PDT, Eric Seidel (no email)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2008-03-14 13:20:01 PDT
_NPN_IntFromIdentifier is wrong

int32_t _NPN_IntFromIdentifier(NPIdentifier identifier)
{
    PrivateIdentifier* i = (PrivateIdentifier*)identifier;
    if (!i->isString)
        return 0;
    return i->value.number;
}


The check should be if (i->isString) instead of !i->isString

This is a one line fix.  The test case is the PITA.
Comment 1 Eric Seidel (no email) 2008-03-14 13:20:43 PDT
Created attachment 19773 [details]
First pass at identifier -> int fix and test case

 .../netscape-identifier-conversion-expected.txt    |   10 ++++
 .../plugins/netscape-identifier-conversion.html    |   13 +++++
 LayoutTests/plugins/resources/TEMPLATE.html        |   13 +++++
 .../resources/netscape-identifier-conversion.js    |   33 +++++++++++
 WebCore/bridge/npruntime.cpp                       |    2 +-
 .../TestNetscapePlugIn.subproj/PluginObject.cpp    |   58 +++++++++++++++++++-
 6 files changed, 126 insertions(+), 3 deletions(-)
Comment 2 Eric Seidel (no email) 2008-03-14 13:21:55 PDT
I don't think I got the test case quite right, debugging now.  I'm posting this now to make sure it doesn't get forgotten.
Comment 3 Eric Seidel (no email) 2008-03-17 13:59:53 PDT
Created attachment 19847 [details]
Cleanup PluginObject.cpp and fix test methods

 .../TestNetscapePlugIn.subproj/PluginObject.cpp    |  473 +++++++++++---------
 1 files changed, 270 insertions(+), 203 deletions(-)
Comment 4 Eric Seidel (no email) 2008-03-17 13:59:54 PDT
Created attachment 19848 [details]
Fix NPAPI implementation and plugin test

 .../netscape-identifier-conversion-expected.txt    |   13 ++++-
 .../resources/netscape-identifier-conversion.js    |   36 ++++-------
 WebCore/WebCore.NPAPI.exp                          |    1 +
 WebKit/mac/Plugins/WebNetscapePluginPackage.m      |    2 +
 .../TestNetscapePlugIn.subproj/PluginObject.cpp    |   66 ++++++++++++--------
 5 files changed, 68 insertions(+), 50 deletions(-)
Comment 5 Eric Seidel (no email) 2008-03-17 14:08:48 PDT
Created attachment 19849 [details]
First pass at identifier -> int fix and test case


Cleanup PluginObject.cpp and fix test methods

Fix NPAPI implementation and plugin test
---
 .../netscape-identifier-conversion-expected.txt    |   21 +
 .../plugins/netscape-identifier-conversion.html    |   13 +
 LayoutTests/plugins/resources/TEMPLATE.html        |   13 +
 .../resources/netscape-identifier-conversion.js    |   23 +
 WebCore/WebCore.NPAPI.exp                          |    1 +
 WebCore/bridge/npruntime.cpp                       |    2 +-
 WebKit/mac/Plugins/WebNetscapePluginPackage.m      |    2 +
 .../TestNetscapePlugIn.subproj/PluginObject.cpp    |  547 ++++++++++++--------
 8 files changed, 415 insertions(+), 207 deletions(-)
Comment 6 Eric Seidel (no email) 2008-03-17 14:11:07 PDT
In order to maintain my sanity, I had to clean up PluginObject a little as well.  I did not make any attempt to further validate our NPNetscapeFuncs struct, it's possible we're still missing other function pointers.
Comment 7 Darin Adler 2008-03-17 14:22:10 PDT
Comment on attachment 19849 [details]
First pass at identifier -> int fix and test case

r=me, all good except for lack of ChangeLog
Comment 8 Eric Seidel (no email) 2008-03-17 14:47:55 PDT
Added changelog and landed as r31112