My suggestion is to add a method returning NSAppleEventDescriptor to WebView, and to make Safari call it. Does this sound right?
Created attachment 6223 [details]
This adds conversion for Boolean, String, Number and Date, and implicitly calls toString() for unknown object types.
A script that I used for testing:
tell application "Safari"
Great idea! Coercing to list descriptors is easy to do using [NSAppleEventDescriptor listDescriptor] and insertDescriptor:atIndex:. Unfortunatly you can only make AppleScript records from dicrionaries with string keys if you drop down to Carbon to contruct them.
Comment on attachment 6223 [details]
The patch looks good, only a few minor details.
The WebView.h header can't be changed until we get the API approved. You will need to use WebViewPrivate.h for this interface.
Created attachment 6237 [details]
(In reply to comment #5)
> I would fall back to always returning a string in the "default" case instead of
> The WebView.h header can't be changed until we get the API approved. You will
> need to use WebViewPrivate.h for this interface.
> Coercing to list descriptors is easy to do using
> [NSAppleEventDescriptor listDescriptor] and insertDescriptor:atIndex:.
Still, one option to consider: only convert arrays that are not sparse, for which element indices start with zero, and don't descend into them recursively (to avoid circular references). What do you think?
Comment on attachment 6237 [details]
I'd like Tim Hatcher to review this, but I have a comment:
(In reply to comment #7)
> This would prevent us from having to expose bool_object.h as a Private header
Ouch, I thought that bool_object.h not being exposed was unintentional (the other *_object.h are already exposed).
(In reply to comment #8)
> Ouch, I thought that bool_object.h not being exposed was unintentional (the
> other *_object.h are already exposed).
On the one hand, it's not mportant that bool_object.h not be exposed. On the other hand, those are details I'd prefer not be depended on by WebCore. I'd like to study why those other headers are needed in WebCore.
Created attachment 6267 [details]
Looks like I was doing the object->value convertion in a needlessly complex way, and toPrimitive() already takes care of this stuff. However, I don't really see where an isDate flag could be added, and if it would be less of an implementation detail than DateInstance::info.
Comment on attachment 6267 [details]
Please don't put a copy of tryGetAndCallProperty into WebCore. And if you feel you must put a copy of it there, use ALWAYS_INLINE, not another copy of that ifdef. Since you're using this specifically just to get a number, the function can be simplified to handle just that case and leave out the type() stuff. The code really needs to check exceptions, and not look at the return value if there's an exception, so it's a pasted copy of incorrect code!
On a separate note, one way to recognize the date class is by its class name, which would avoid the need to include DateInstance and link to DateInstance::info. You can just say if o->className() == "Date".
How are you testing this function? I think we need to hook this up to DumpRenderTree so we can test that it's working.
(In reply to comment #11)
> How are you testing this function? I think we need to hook this up to
> DumpRenderTree so we can test that it's working.
I just had some custom code for debugging. Using DumpRenderTree is indeed so much better; I've added code for this and a test - will be available in the next revision.
Created attachment 6296 [details]
Besides the already discussed improvements, changed the way how milliseconds are converted to LongDateTime - the new code works for dates in the past, where POSIX and Mac date routines disagree about time zone handling.
Comment on attachment 6296 [details]
Good idea about the LayoutTests. This looks good. r=me
Tweaked the date test when landing - made it timezone-agnostic.
Filed an enchancement request for Safari to use this new API: <rdar://4435959>.