Bug 88411 - FileAPI: If type consists of non-ASCII characters in Blob constructor, it should throw a SyntaxError.
Summary: FileAPI: If type consists of non-ASCII characters in Blob constructor, it sho...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Li Yin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-06 06:30 PDT by Li Yin
Modified: 2012-06-07 04:07 PDT (History)
8 users (show)

See Also:


Attachments
Patch (5.50 KB, patch)
2012-06-06 06:59 PDT, Li Yin
no flags Details | Formatted Diff | Diff
Patch (5.59 KB, patch)
2012-06-06 07:34 PDT, Li Yin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (220.75 KB, application/zip)
2012-06-06 18:03 PDT, WebKit Review Bot
no flags Details
Patch (5.55 KB, patch)
2012-06-06 21:26 PDT, Li Yin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Li Yin 2012-06-06 06:30:50 PDT
From Spec: http://dev.w3.org/2006/webapi/FileAPI/#constructorBlob
If type consists of any non-ASCII characters, throw a SyntaxError and return from this algorithm.
Comment 1 Li Yin 2012-06-06 06:59:14 PDT
Created attachment 146013 [details]
Patch
Comment 2 Kentaro Hara 2012-06-06 07:07:45 PDT
Comment on attachment 146013 [details]
Patch

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

> Source/WebCore/bindings/js/JSBlobCustom.cpp:108
> +            setDOMException(exec, SYNTAX_ERR);
> +            return JSValue::encode(JSValue());

SYNTAX_ERR and SyntaxError are different. You need to use 'return throwVMError(exec, createSyntaxError(exec, "..."));'

> Source/WebCore/bindings/v8/custom/V8BlobCustom.cpp:106
> +            return throwError(SYNTAX_ERR, args.GetIsolate());

Ditto. You need to use 'return V8Proxy::throwError(V8Proxy::SyntaxError, "...");'
Comment 3 Li Yin 2012-06-06 07:34:53 PDT
Created attachment 146024 [details]
Patch
Comment 4 Li Yin 2012-06-06 07:36:20 PDT
(In reply to comment #2)
> (From update of attachment 146013 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146013&action=review
> 
> > Source/WebCore/bindings/js/JSBlobCustom.cpp:108
> > +            setDOMException(exec, SYNTAX_ERR);
> > +            return JSValue::encode(JSValue());
> 
> SYNTAX_ERR and SyntaxError are different. You need to use 'return throwVMError(exec, createSyntaxError(exec, "..."));'
> 
> > Source/WebCore/bindings/v8/custom/V8BlobCustom.cpp:106
> > +            return throwError(SYNTAX_ERR, args.GetIsolate());
> 
> Ditto. You need to use 'return V8Proxy::throwError(V8Proxy::SyntaxError, "...");'

Thanks for your review.
Comment 5 Kentaro Hara 2012-06-06 07:40:58 PDT
Comment on attachment 146024 [details]
Patch

The patch looks OK. I think there is no compatibility concern (Firefox and IE do not implement the Blob constructor) but let's wait for a comment from File API folks.

kinuko-san: Does this change look OK?
Comment 6 WebKit Review Bot 2012-06-06 18:03:27 PDT
Comment on attachment 146024 [details]
Patch

Attachment 146024 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12917024

New failing tests:
canvas/philip/tests/2d.gradient.radial.cone.top.html
canvas/philip/tests/2d.gradient.radial.cone.shape2.html
Comment 7 WebKit Review Bot 2012-06-06 18:03:31 PDT
Created attachment 146166 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 8 Li Yin 2012-06-06 20:04:53 PDT
It is so strange. 
This patch can result in 2d.gradient.radial.cone.top.html test failed.
Comment 9 Li Yin 2012-06-06 21:26:32 PDT
Created attachment 146190 [details]
Patch
Comment 10 Li Yin 2012-06-06 21:32:59 PDT
Rebase to the newest code, the followed tests passed on chromium gtk port.
canvas/philip/tests/2d.gradient.radial.cone.top.html
canvas/philip/tests/2d.gradient.radial.cone.shape2.html

Upload the patch again.
Comment 11 Li Yin 2012-06-07 00:43:01 PDT
Hi kinuko,
  Could you have a look? Thanks.
Comment 12 Kinuko Yasuda 2012-06-07 03:27:21 PDT
Comment on attachment 146190 [details]
Patch

The patch looks good to me (if the binding part lg to haraken@).
Comment 13 Kentaro Hara 2012-06-07 03:28:04 PDT
Comment on attachment 146190 [details]
Patch

thanks kinuko-san!
Comment 14 WebKit Review Bot 2012-06-07 04:07:21 PDT
Comment on attachment 146190 [details]
Patch

Clearing flags on attachment: 146190

Committed r119702: <http://trac.webkit.org/changeset/119702>
Comment 15 WebKit Review Bot 2012-06-07 04:07:44 PDT
All reviewed patches have been landed.  Closing bug.