Bug 13129 - Democracy Player dies in NSException from WebScriptObject on startup
Summary: Democracy Player dies in NSException from WebScriptObject on startup
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.5
: P2 Major
Assignee: Nobody
URL: http://www.getdemocracy.com/
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-03-20 06:31 PDT by Mark Rowe (bdash)
Modified: 2011-03-09 00:02 PST (History)
2 users (show)

See Also:


Attachments
Patch (3.20 KB, patch)
2007-03-20 07:02 PDT, Mark Rowe (bdash)
no flags Details | Formatted Diff | Diff
Hide "count" at all times in an explicit fashion (3.23 KB, patch)
2007-03-31 03:12 PDT, Mark Rowe (bdash)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 2007-03-20 06:31:18 PDT
Democracy Player is a media player written in Python using PyObjC.  It's interface makes heavy use of WebKit.  With a ToT build of WebKit Democracy player throws an NSException on launch and fails to function.  This also means that Democracy Player does not work at all in Leopard.  The exception is as follows:

KeyError: 'NSUnknownKeyException - [<DOMHTMLElement 0x1622ef90> valueForUndefinedKey:]: this class is not key value coding-compliant for the key length.'

The cause for this is Python code such as:

        someElement = document.getElementsByID_('someElement')
        if not someElement:
            # do stuff here

The "not someElement" attempts to coerce Python's view on the DOMHTMLElement object into a boolean.  I'm not completely sure what logic it uses to do this, but one step seems to be checking whether the object is sequence-like.  From an Objective-C point of view this means implementing -(int)count.  *All* WebScriptObjects implement this method as of r13877, with the big downside that it throws an exception if the WebScriptObject is not KVC-compliant for "length".  One solution would be to only expose the "count" method if this object is KVC-compliant for "length".
Comment 1 Mark Rowe (bdash) 2007-03-20 07:02:08 PDT
Created attachment 13716 [details]
Patch
Comment 2 Timothy Hatcher 2007-03-20 07:24:38 PDT
Comment on attachment 13716 [details]
Patch

Nice solution! You should really just use @try/@catch instead of NS_DURING/NS_HANDLER. Those macros just wrap @try now...
Comment 3 Mark Rowe (bdash) 2007-03-20 08:26:34 PDT
Landed in r20343.
Comment 4 Darin Adler 2007-03-30 21:49:46 PDT
This change broke Drosera. See bug 13239 for details.
Comment 5 Darin Adler 2007-03-30 22:40:34 PDT
The patch landed here had incorrect selectors in it:

    count: and _count: rather than count and _count

And we didn't test that the count method still works. It doesn't! In fact, the _shouldRespondToCount method is never called.
Comment 6 Mark Rowe (bdash) 2007-03-31 03:10:50 PDT
As Darin points out my "fix" was bogus.  It fixed Democracy Player by hiding "count" at all times.
Comment 7 Mark Rowe (bdash) 2007-03-31 03:12:33 PDT
Created attachment 13903 [details]
Hide "count" at all times in an explicit fashion

This provides the same behaviour that ToT has but in an explicit manner.  I haven't been able to determine anywhere, other than in Drosera, that -[WebScriptObject count] is called.
Comment 8 Timothy Hatcher 2007-03-31 12:12:44 PDT
Count and objectAtIndex: was added to better support bindings. Fixed in bug 8389.

Does Democracy Player blow up on the presence of count? Or the fact that calling count throws an exception?

We could simply put a @try/@catch in count to prevent the exception. We could also call lower level JavaScriptCore methods to see if the script object has the count property first. That would be faster than a @try/@catch.
Comment 9 Darin Adler 2007-03-31 16:23:05 PDT
Comment on attachment 13903 [details]
Hide "count" at all times in an explicit fashion

r=me
Comment 10 Mark Rowe (bdash) 2007-03-31 18:26:07 PDT
Tim, "count" is being called implicitly by PyObjC when Democracy does "if myDOMNode:".  The fact it throws an exception makes this a very visible problem.  If "count" were to *not* throw an exception what would it return when the "length" property is not present?  0 or nil would be the two obvious values, but both of these would make "myDOMNode" appear as boolean false to Python and would break Democracy in a more subtle manner.

It's not clear to me how "count" helps for bindings support.  If it is required we might want to wrap collection-like JS objects in a subclass of WebScriptObject which implements count/objectAtIndex: rather than exposing them on *all* JS objects?
Comment 11 Mark Rowe (bdash) 2007-03-31 22:06:27 PDT
Landed in r20642.
Comment 12 Mark Rowe (bdash) 2007-03-31 22:15:04 PDT
<rdar://problem/5103228>
Comment 13 Eric Seidel (no email) 2011-03-09 00:02:30 PST
There is some #if 0 code in WebCore which references this bug:

#if 0 

// FIXME: We'd like to add this, but we can't do that until this issue is resolved:
// http://bugs.webkit.org/show_bug.cgi?id=13129: presence of 'count' method on
// WebScriptObject breaks Democracy player.

- (unsigned)count
{
    id length = [self valueForKey:@"length"];
    if (![length respondsToSelector:@selector(intValue)])
        return 0;
    return [length intValue];
}

#endif

Can we turn that on now?