Bug 13129 – Democracy Player dies in NSException from WebScriptObject on startup

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Mark Rowe (bdash)]

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".

[Mark Rowe (bdash)]

Created attachment 13716 [details]
Patch

[Timothy Hatcher]

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...

[Mark Rowe (bdash)]

Landed in r20343.

[Darin Adler]

This change broke Drosera. See bug 13239 for details.

[Darin Adler]

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.

[Mark Rowe (bdash)]

As Darin points out my "fix" was bogus.  It fixed Democracy Player by hiding "count" at all times.

[Mark Rowe (bdash)]

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.

[Timothy Hatcher]

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.

[Darin Adler]

Comment on attachment 13903 [details]
Hide "count" at all times in an explicit fashion

r=me

[Mark Rowe (bdash)]

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?

[Mark Rowe (bdash)]

Landed in r20642.

[Mark Rowe (bdash)]

<rdar://problem/5103228>

[Eric Seidel (no email)]

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?