Bug 113518 - use XMLHttpRequestResponseType enumeration in XMLHttpRequest.idl
Summary: use XMLHttpRequestResponseType enumeration in XMLHttpRequest.idl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: arno.
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-28 10:45 PDT by arno.
Modified: 2013-03-28 15:38 PDT (History)
8 users (show)

See Also:


Attachments
patch proposal (4.31 KB, patch)
2013-03-28 11:00 PDT, arno.
no flags Details | Formatted Diff | Diff
updated patch: comment json in idl + tests all valid types in xmlhttprequest-responsetype-set-invalidtype (5.43 KB, patch)
2013-03-28 14:24 PDT, arno.
no flags Details | Formatted Diff | Diff
updated patch: rename xmlhttprequest-responsetype-set-invalidtype to xmlhttprequest-set-responsetype (8.08 KB, patch)
2013-03-28 14:35 PDT, arno.
no flags Details | Formatted Diff | Diff
updated patch: udpate xmlhttprequest-set-responsetype-expected; hopefully this one is good (8.26 KB, patch)
2013-03-28 14:45 PDT, arno.
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description arno. 2013-03-28 10:45:13 PDT
Hi,
since idl enumerations are now supported, we can now use XMLHttpRequestResponseType for XMLHttpRequest responseType.
Comment 1 arno. 2013-03-28 11:00:15 PDT
Created attachment 195600 [details]
patch proposal
Comment 2 Kentaro Hara 2013-03-28 11:10:24 PDT
Comment on attachment 195600 [details]
patch proposal

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

> Source/WebCore/ChangeLog:16
> +        Already covered by
> +        xmlhttprequest-responsetype-set-invalidtype-expected.txt

Is there any test for valid types?

> Source/WebCore/xml/XMLHttpRequest.idl:34
> +    "json",

I thought "json" is not supported: https://bugs.webkit.org/show_bug.cgi?id=73648

Maybe it would be better to comment out "json" with a FIXME.
Comment 3 arno. 2013-03-28 14:24:24 PDT
Created attachment 195641 [details]
updated patch: comment json in idl + tests all valid types in xmlhttprequest-responsetype-set-invalidtype
Comment 4 Kentaro Hara 2013-03-28 14:30:31 PDT
Comment on attachment 195641 [details]
updated patch: comment json in idl + tests all valid types in xmlhttprequest-responsetype-set-invalidtype

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

Looks OK with nits.

> LayoutTests/ChangeLog:12
> +        * fast/xmlhttprequest/xmlhttprequest-responsetype-set-invalidtype-expected.txt:

Let's rename the test to xmlhttprequest-set-responsetype.html, as we're now testing valid types as well.
Comment 5 arno. 2013-03-28 14:35:42 PDT
Created attachment 195645 [details]
updated patch: rename xmlhttprequest-responsetype-set-invalidtype to xmlhttprequest-set-responsetype
Comment 6 Kentaro Hara 2013-03-28 14:39:00 PDT
Comment on attachment 195645 [details]
updated patch: rename xmlhttprequest-responsetype-set-invalidtype to xmlhttprequest-set-responsetype

LGTM
Comment 7 Kentaro Hara 2013-03-28 14:40:00 PDT
Comment on attachment 195645 [details]
updated patch: rename xmlhttprequest-responsetype-set-invalidtype to xmlhttprequest-set-responsetype

Oops, it looks like you forgot to update xmlhttprequest-set-responsetype-expected.txt.
Comment 8 arno. 2013-03-28 14:45:54 PDT
Created attachment 195648 [details]
updated patch: udpate xmlhttprequest-set-responsetype-expected; hopefully this one is good
Comment 9 Kentaro Hara 2013-03-28 15:12:09 PDT
Comment on attachment 195648 [details]
updated patch: udpate xmlhttprequest-set-responsetype-expected; hopefully this one is good

Looks good.
Comment 10 WebKit Review Bot 2013-03-28 15:38:03 PDT
Comment on attachment 195648 [details]
updated patch: udpate xmlhttprequest-set-responsetype-expected; hopefully this one is good

Clearing flags on attachment: 195648

Committed r147172: <http://trac.webkit.org/changeset/147172>
Comment 11 WebKit Review Bot 2013-03-28 15:38:07 PDT
All reviewed patches have been landed.  Closing bug.