Bug 58516

Summary:

Have canvas drawImageFromRect just redirect to use drawImage

Product:

WebKit

Reporter:

Matthew Delaney <mdelaney7>

Component:

Canvas

Assignee:

Matthew Delaney <mdelaney7>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cmarrin, dglazkov, mdelaney7, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	none
Patch	simon.fraser: review+

Description Matthew Delaney 2011-04-13 22:53:50 PDT

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

Comment 1 Matthew Delaney 2011-04-14 11:08:58 PDT

Created attachment 89608 [details]
Patch

Comment 2 Simon Fraser (smfr) 2011-04-14 11:15:38 PDT

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.

Comment 3 WebKit Review Bot 2011-04-14 11:21:16 PDT

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

Comment 4 Matthew Delaney 2011-04-14 11:48:14 PDT

Created attachment 89616 [details]
Patch

Comment 5 Matthew Delaney 2011-04-14 11:50:48 PDT

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

Comment 6 WebKit Review Bot 2011-04-14 12:11:39 PDT

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

Comment 7 Matthew Delaney 2011-04-14 13:31:54 PDT

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