Bug 25891 - _NPN_HasMethod is too liberal in declaring methods
Summary: _NPN_HasMethod is too liberal in declaring methods
Status: RESOLVED LATER
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Holger Freyther
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-20 09:08 PDT by Holger Freyther
Modified: 2009-07-16 22:18 PDT (History)
0 users

See Also:


Attachments
LayoutTest to illustrate the current behavior (2.86 KB, patch)
2009-05-24 01:24 PDT, Holger Freyther
no flags Details | Formatted Diff | Diff
Test Case + SourceCode change + hopefully better ChangeLog entry (4.39 KB, patch)
2009-05-25 00:49 PDT, Holger Freyther
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Holger Freyther 2009-05-20 09:08:14 PDT
The current implementation is declaring every property that is !== undefined as a method. This should be changed.
Comment 1 Holger Freyther 2009-05-24 01:24:33 PDT
Created attachment 30626 [details]
LayoutTest to illustrate the current behavior

With this layout test a null property and a string property will be declared as method by the _NPN_HasMethod function.
Comment 2 Maciej Stachowiak 2009-05-24 03:45:59 PDT
Comment on attachment 30626 [details]
LayoutTest to illustrate the current behavior

It's fine to land this test,  but maybe note that the current behavior is buggy and will be fixed soon, with reference to this bug.
Comment 3 Holger Freyther 2009-05-25 00:49:42 PDT
Created attachment 30646 [details]
Test Case + SourceCode change + hopefully better ChangeLog entry

This time also with changing the implementation of the method. I assume anders will provide comments and I should try to run the test within firefox to verify that behavior.
Comment 4 Maciej Stachowiak 2009-05-25 01:20:06 PDT
Comment on attachment 30646 [details]
Test Case + SourceCode change + hopefully better ChangeLog entry

r=me if this matches Firefox behavior (please test).
Comment 5 David Levin 2009-05-29 11:32:52 PDT
Assign to zecke for landing to make it more clear in the commit queue (and there is something to verify before landing as well).
Comment 6 Holger Freyther 2009-05-29 19:40:01 PDT
I had some difficulties in getting our example plugin loaded in ff. I will try it this weekend.
Comment 7 Eric Seidel (no email) 2009-06-23 16:12:49 PDT
Comment on attachment 30646 [details]
Test Case + SourceCode change + hopefully better ChangeLog entry

Fails to build:

Ld /Users/eseidel/Projects/build/Debug/WebCore.framework/Versions/A/WebCore normal i386
    cd /Users/eseidel/Projects/WebKit/WebCore
    setenv MACOSX_DEPLOYMENT_TARGET 10.5
    /Developer/usr/bin/g++-4.2 -arch i386 -dynamiclib -L/Users/eseidel/Projects/build/Debug -F/Users/eseidel/Projects/build/Debug -F/System/Library/Frameworks/Carbon.framework/Frameworks -F/System/Library/Frameworks/ApplicationServices.framework/Frameworks -filelist /Users/eseidel/Projects/build/WebCore.build/Debug/WebCore.build/Objects-normal/i386/WebCore.LinkFileList -Wl,--no-demangle -exported_symbols_list /Users/eseidel/Projects/build/Debug/DerivedSources/WebCore/WebCore.exp -install_name /Users/eseidel/Projects/build/Debug/WebCore.framework/Versions/A/WebCore -mmacosx-version-min=10.5 -lWebCoreSQLite3 -lobjc -sub_library libobjc -umbrella WebKit -framework ApplicationServices -framework Carbon -framework Cocoa -framework JavaScriptCore -framework QuartzCore -framework SystemConfiguration -licucore -lobjc -lxml2 -Wl,-single_module -compatibility_version 1 -current_version 531.0 -o /Users/eseidel/Projects/build/Debug/WebCore.framework/Versions/A/WebCore
Undefined symbols:
  "__ZN3JSC16jsIsFunctionTypeENS_7JSValueE", referenced from:
      __NPN_HasMethod in NP_jsobject.o
ld: symbol(s) not found
Comment 8 Holger Freyther 2009-06-23 16:21:04 PDT
Thanks for giving it a try. The build issue can be easily fixed (even after landing) the more serious problem is that first attempt to try the plugin + testcase in firefox failed... maybe you want to give it a try?
Comment 9 Eric Seidel (no email) 2009-06-24 01:08:11 PDT
Comment on attachment 30626 [details]
LayoutTest to illustrate the current behavior

Clearing r+ on this obsolete patch.
Comment 10 Holger Freyther 2009-06-25 00:24:57 PDT
I managed to get the plugin built and tested on firefox... And we are not too liberal... in contrast we are not liberal enough. In WebKit hasmethod on a property with undefined value returns false but in firefox it returns true...

should we bother? should we close this bug as invalid?
Comment 11 Holger Freyther 2009-07-16 22:18:30 PDT
We are not aware of any issues this minor incompability is causing, so close the bug as LATER/WORKSFORSOME.