Bug 78328 - Remove [ConvertingNullStringTo] from CloseEvent.idl
Summary: Remove [ConvertingNullStringTo] from CloseEvent.idl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 77393
  Show dependency treegraph
 
Reported: 2012-02-09 23:25 PST by Kentaro Hara
Modified: 2012-02-14 15:39 PST (History)
4 users (show)

See Also:


Attachments
Patch (4.39 KB, patch)
2012-02-09 23:32 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (1.98 KB, patch)
2012-02-10 00:04 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.