Bug 58516 - Have canvas drawImageFromRect just redirect to use drawImage
Summary: Have canvas drawImageFromRect just redirect to use drawImage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Matthew Delaney
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-13 22:53 PDT by Matthew Delaney
Modified: 2011-04-14 13:31 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.79 KB, patch)
2011-04-14 11:08 PDT, Matthew Delaney
no flags Details | Formatted Diff | Diff
Patch (7.00 KB, patch)
2011-04-14 11:48 PDT, Matthew Delaney
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>