Bug 113518

Summary: use XMLHttpRequestResponseType enumeration in XMLHttpRequest.idl
Product: WebKit Reporter: arno. <a.renevier>
Component: WebKit Misc.Assignee: arno. <a.renevier>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, crogers, esprehn+autocc, haraken, nbarth, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch proposal
none
updated patch: comment json in idl + tests all valid types in xmlhttprequest-responsetype-set-invalidtype
none
updated patch: rename xmlhttprequest-responsetype-set-invalidtype to xmlhttprequest-set-responsetype
none
updated patch: udpate xmlhttprequest-set-responsetype-expected; hopefully this one is good none

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.