Bug 25891 – _NPN_HasMethod is too liberal in declaring methods

• Component: Plug-ins
• Status: RESOLVED LATER

[Holger Freyther]

The current implementation is declaring every property that is !== undefined as a method. This should be changed.

[Holger Freyther]

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.

[Maciej Stachowiak]

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.

[Holger Freyther]

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.

[Maciej Stachowiak]

Comment on attachment 30646 [details]
Test Case + SourceCode change + hopefully better ChangeLog entry

r=me if this matches Firefox behavior (please test).

[David Levin]

Assign to zecke for landing to make it more clear in the commit queue (and there is something to verify before landing as well).

[Holger Freyther]

I had some difficulties in getting our example plugin loaded in ff. I will try it this weekend.

[Eric Seidel (no email)]

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

[Holger Freyther]

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?

[Eric Seidel (no email)]

Comment on attachment 30626 [details]
LayoutTest to illustrate the current behavior

Clearing r+ on this obsolete patch.

[Holger Freyther]

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?

[Holger Freyther]

We are not aware of any issues this minor incompability is causing, so close the bug as LATER/WORKSFORSOME.