Bug 58516 – Have canvas drawImageFromRect just redirect to use drawImage

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Matthew Delaney]

Currently drawImageFromRect re-implements pretty much everything drawImage does already for it. It should just use drawImage directly instead.

[Matthew Delaney]

Created attachment 89608 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 89608 [details]
Patch

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

> Source/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp:299
> -                               ustringToString(exec->argument(9).toString(exec)));    
> -    return jsUndefined();    
> +                               ustringToString(exec->argument(9).toString(exec)), ec);
> +    setDOMException(exec, ec);
> +
> +    return jsUndefined();

This change looks unrelated. If you did intend to include it, then the changelog should describe it, and the testcase should exercise it.

[WebKit Review Bot]

Attachment 89608 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8397690

[Matthew Delaney]

Created attachment 89616 [details]
Patch

[Matthew Delaney]

> > Source/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp:299
> > -                               ustringToString(exec->argument(9).toString(exec)));    
> > -    return jsUndefined();    
> > +                               ustringToString(exec->argument(9).toString(exec)), ec);
> > +    setDOMException(exec, ec);
> > +
> > +    return jsUndefined();
> 
> This change looks unrelated. If you did intend to include it, then the changelog should describe it, and the testcase should exercise it.

Good call. I initially thought it would be best for drawImageFromRect to behave just as drawImage does WRT dom exceptions and such. Though, since it's simply a legacy routine used by only a few sites and dashboard widgets, I think it's best to leave its behavior as-is. Updated patch does so.

[WebKit Review Bot]

Attachment 89608 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8400761

[Matthew Delaney]

Committed r83888: <http://trac.webkit.org/changeset/83888>