Bug 82045 - The JSC code generator can't handle boolean arguments for Callbacks
Summary: The JSC code generator can't handle boolean arguments for Callbacks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tommy Widenflycht
URL:
Keywords:
Depends on:
Blocks: 81657
  Show dependency tree / graph
 
Reported: 2012-03-23 04:23 PDT by Tommy Widenflycht
Modified: 2012-03-23 06:01 PDT (History)
5 users (show)

See Also:

Attachments
Patch (1.67 KB, patch)
2012-03-23 04:36 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Patch (11.79 KB, patch)
2012-03-23 05:10 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tommy Widenflycht 2012-03-23 04:23:18 PDT 
CodeGeneratorJS.pm only handles DOMStrings and objects as arguments for a Callback, so I added support for boolean values as well.
Comment 1 Tommy Widenflycht 2012-03-23 04:36:54 PDT 
Created attachment 133459 [details]
Patch
Comment 2 Kentaro Hara 2012-03-23 04:41:58 PDT 
Comment on attachment 133459 [details]
Patch

OK!
Comment 3 Kentaro Hara 2012-03-23 04:42:40 PDT 
Comment on attachment 133459 [details]
Patch

Oops, would you please add run-bindings-tests? https://trac.webkit.org/wiki/WebKitIDL#RunBindingsTests
Comment 4 Tommy Widenflycht 2012-03-23 05:10:28 PDT 
Created attachment 133463 [details]
Patch
Comment 5 Kentaro Hara 2012-03-23 05:18:20 PDT 
Comment on attachment 133463 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133463&action=review

> Source/WebCore/bindings/scripts/test/TestCallback.idl:42
> +      boolean callbackWithBoolean(in boolean boolParam);

Sorry for my wrong comment in IRC. This does not test your change, since TestObj does not have the [Callback] IDL attribute. You need to add this test case to TestCallback.idl.
Comment 6 Tommy Widenflycht 2012-03-23 05:25:46 PDT 
Comment on attachment 133463 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133463&action=review

>> Source/WebCore/bindings/scripts/test/TestCallback.idl:42
>> +      boolean callbackWithBoolean(in boolean boolParam);
> 
> Sorry for my wrong comment in IRC. This does not test your change, since TestObj does not have the [Callback] IDL attribute. You need to add this test case to TestCallback.idl.

This IS in TestCallback.idl...
Comment 7 Kentaro Hara 2012-03-23 05:27:23 PDT 
Comment on attachment 133463 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133463&action=review

>>> Source/WebCore/bindings/scripts/test/TestCallback.idl:42
>>> +      boolean callbackWithBoolean(in boolean boolParam);
>> 
>> Sorry for my wrong comment in IRC. This does not test your change, since TestObj does not have the [Callback] IDL attribute. You need to add this test case to TestCallback.idl.
> 
> This IS in TestCallback.idl...

Sorry for the repeated confusion:)
Comment 8 WebKit Review Bot 2012-03-23 06:01:51 PDT 
Comment on attachment 133463 [details]
Patch

Clearing flags on attachment: 133463

Committed r111853: <http://trac.webkit.org/changeset/111853>
Comment 9 WebKit Review Bot 2012-03-23 06:01:56 PDT 
All reviewed patches have been landed.  Closing bug.