Bug 88411 – FileAPI: If type consists of non-ASCII characters in Blob constructor, it should throw a SyntaxError.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Li Yin]

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.

[Li Yin]

Created attachment 146013 [details]
Patch

[Kentaro Hara]

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, "...");'

[Li Yin]

Created attachment 146024 [details]
Patch

[Li Yin]

(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.

[Kentaro Hara]

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?

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Li Yin]

It is so strange. 
This patch can result in 2d.gradient.radial.cone.top.html test failed.

[Li Yin]

Created attachment 146190 [details]
Patch

[Li Yin]

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.

[Li Yin]

Hi kinuko,
  Could you have a look? Thanks.

[Kinuko Yasuda]

Comment on attachment 146190 [details]
Patch

The patch looks good to me (if the binding part lg to haraken@).

[Kentaro Hara]

Comment on attachment 146190 [details]
Patch

thanks kinuko-san!

[WebKit Review Bot]

Comment on attachment 146190 [details]
Patch

Clearing flags on attachment: 146190

Committed r119702: <http://trac.webkit.org/changeset/119702>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.