Bug 78328

Summary: Remove [ConvertingNullStringTo] from CloseEvent.idl
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: WebCore JavaScriptAssignee: Kentaro Hara <haraken>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77393    
Attachments:
Description Flags
Patch
none
Patch none

Description Kentaro Hara 2012-02-09 23:25:07 PST 
In CloseEvent.idl, [ConvertingNullStringTo] is a typo of [ConvertNullStringTo]. In bug 78108, [ConvertNullStringTo] was renamed to [TreatReturnedNullStringAs].

In conclusion, we should replace [ConvertingNullStringTo] with [TreatReturnedNullStringAs].
Comment 1 Kentaro Hara 2012-02-09 23:32:54 PST 
Created attachment 126464 [details]
Patch
Comment 2 Adam Barth 2012-02-09 23:37:28 PST 
Comment on attachment 126464 [details]
Patch

Have you cross-checked this with the spec?
Comment 3 Kentaro Hara 2012-02-10 00:04:37 PST 
Created attachment 126467 [details]
Patch
Comment 4 Kentaro Hara 2012-02-10 00:05:23 PST 
(In reply to comment #2)
> (From update of attachment 126464 [details])
> Have you cross-checked this with the spec?

The spec says "The reason attribute must return the value it was initialized to. When the object is created, this attribute must be initialized to empty string.".
http://dev.w3.org/html5/websockets/#event-definitions

So we should simply remove [ConvertingNullStringTo]. Uploaded a revised patch.
Comment 5 WebKit Review Bot 2012-02-10 01:28:23 PST 
Comment on attachment 126467 [details]
Patch

Clearing flags on attachment: 126467

Committed r107382: <http://trac.webkit.org/changeset/107382>
Comment 6 WebKit Review Bot 2012-02-10 01:28:27 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Darin Adler 2012-02-13 15:31:58 PST 
All sounds fine, but what about test coverage?
Comment 8 Kentaro Hara 2012-02-13 16:01:34 PST 
(In reply to comment #7)
> All sounds fine, but what about test coverage?

Darin: The following existing test is in close-event-constructor.html:

    shouldBeEqualToString("new CloseEvent('eventType', { reason: '' }).reason", "");

Fortunately, we didn't need to change the test result (since [ConvertingNullStringTo] was typo and had not been working at all).
Comment 9 Darin Adler 2012-02-14 15:33:14 PST 
(In reply to comment #8)
> Fortunately, we didn't need to change the test result

So if we had added the correctly spelled attribute the test would have failed?
Comment 10 Kentaro Hara 2012-02-14 15:39:14 PST 
(In reply to comment #9)
> (In reply to comment #8)
> > Fortunately, we didn't need to change the test result
> 
> So if we had added the correctly spelled attribute the test would have failed?

Yes.