Summary: | FileAPI: type should be converted to be lower case in Blob constructor. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Li Yin <li.yin> | ||||||
Component: | WebCore Misc. | Assignee: | Li Yin <li.yin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, haraken, japhet, jianli, jochen, kinuko, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Li Yin
2012-06-08 18:29:16 PDT
Created attachment 146678 [details]
Patch
Comment on attachment 146678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146678&action=review > LayoutTests/ChangeLog:9 > + Spec: http://dev.w3.org/2006/webapi/FileAPI/#dfn-BlobPropertyBag > + Add sub-test to track changing upper case into lower case of type in Blob constructor. I think the spec description might be ambiguous, and I am not sure if we should make this change. IMHO, "Create a Blob with type='CamelCase', then blob.type will return 'camelcase'" sounds strange compared to other constructor behaviors. The spec says as follows: 6. If the optional BlobPropertyBag dictionary is used to invoke the constructor and has a type member, then user agents MUST run the following substeps: ... 2. Convert every character in s to lower case. 3. Parse s as an RFC2616 media-type, tokenizing it according to the ABNF for media-type, ... 7. ... otherwise, set this Blob object's type to s. If we read the spec naively, "CamelCase" should be converted to "camelcase", and then "camelcase" should be set to type. However, ---although this is just my personal opinion---, the spec might be intending the following interpretation: 6. If the optional BlobPropertyBag dictionary is used to invoke the constructor and has a type member, then user agents MUST run the following substeps: ... 2. Convert every character in s to lower case. 3. Parse ***the lower-cased string*** as an RFC2616 media-type, tokenizing it according to the ABNF for media-type, ... 7. ... otherwise, set this Blob object's type to s. In this case, the lower-case conversion is just for obtaining the media-type, and s is still "CamelCase". IMHO, this behavior sounds more reasonable to me. (Correct me if I'm wrong. If you would have the same idea, you can file a spec bug to clarify the intention of the spec.) (In reply to comment #2) > (From update of attachment 146678 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146678&action=review > > > LayoutTests/ChangeLog:9 > > + Spec: http://dev.w3.org/2006/webapi/FileAPI/#dfn-BlobPropertyBag > > + Add sub-test to track changing upper case into lower case of type in Blob constructor. > > I think the spec description might be ambiguous, and I am not sure if we should make this change. IMHO, "Create a Blob with type='CamelCase', then blob.type will return 'camelcase'" sounds strange compared to other constructor behaviors. Maybe I think the spec means the type should not be case-sensitive. That is to say, we use {type: "CamelCase"} or {type: "camelcase"} will get the same result. (This is just my understanding) Thanks for your comments. (In reply to comment #3) > Maybe I think the spec means the type should not be case-sensitive. > That is to say, we use {type: "CamelCase"} or {type: "camelcase"} will get the same result. (This is just my understanding) I agree that the type should be case-unsensitive, but maybe the type attribute should keep the original value that JS specified. Specifically, my (personal) expectation is: new Blob([], {type: "text/html"}); // Recognized as "text/html" new Blob([], {type: "Text/Html"}); // Recognized as "text/html" new Blob([], {type: "text/html"}).type; // "text/html" new Blob([], {type: "Text/Html"}).type; // "Text/Html" new Blob([], {type: "text/html; charset=UTF-8"}).type; // "text/html; charset=UTF-8" It seems a bit strange to me if the result is "text/html; charset=utf-8".(http://www.w3.org/TR/FileAPI/#enctype) (In reply to comment #4) > (In reply to comment #3) > > Maybe I think the spec means the type should not be case-sensitive. > > That is to say, we use {type: "CamelCase"} or {type: "camelcase"} will get the same result. (This is just my understanding) > > I agree that the type should be case-unsensitive, but maybe the type attribute should keep the original value that JS specified. Specifically, my (personal) expectation is: > > new Blob([], {type: "text/html"}); // Recognized as "text/html" > new Blob([], {type: "Text/Html"}); // Recognized as "text/html" > new Blob([], {type: "text/html"}).type; // "text/html" > new Blob([], {type: "Text/Html"}).type; // "Text/Html" > new Blob([], {type: "text/html; charset=UTF-8"}).type; // "text/html; charset=UTF-8" > > It seems a bit strange to me if the result is "text/html; charset=utf-8".(http://www.w3.org/TR/FileAPI/#enctype) Okay, the spec seems unclear for this behavior. Let me file a bug in w3c bugzilla. I filed a bug in W3C bugzilla. https://www.w3.org/Bugs/Public/show_bug.cgi?id=17449 From Spec: http://dev.w3.org/2006/webapi/FileAPI/#attributes-blob The ASCII-encoded string in lower case representing the media type of the Blob. It explicitly specified the type should be lower case. That is to say, it is a webkit bug, we still need to fix it. (In reply to comment #7) > From Spec: http://dev.w3.org/2006/webapi/FileAPI/#attributes-blob > The ASCII-encoded string in lower case representing the media type of the Blob. > > It explicitly specified the type should be lower case. > That is to say, it is a webkit bug, we still need to fix it. Thank you very much for the clarification. Then the latest patch looks OK to me. kinuko-san: Would you take a look? I need to redesign the test. Please wait for my update. Thanks. Created attachment 147225 [details]
Patch
Update the test case, use the correct media-type format. Would you please have a check again? Thanks in advance. LGTM. kinuko-san: review? Comment on attachment 147225 [details]
Patch
LGTM too. Thanks for fixing!
Comment on attachment 147225 [details] Patch Clearing flags on attachment: 147225 Committed r120191: <http://trac.webkit.org/changeset/120191> All reviewed patches have been landed. Closing bug. |