Bug 43341 - Canvas is not reset when setting canvas.width
Summary: Canvas is not reset when setting canvas.width
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Major
Assignee: Matthew Delaney
URL: http://codingwithattitude.net/gml/gml...
Depends on:
Reported: 2010-08-02 05:56 PDT by lvanbass+webkitbugzilla
Modified: 2010-08-14 08:37 PDT (History)
4 users (show)

See Also:

Patch (1.45 KB, patch)
2010-08-03 21:11 PDT, Matthew Delaney
no flags Details | Formatted Diff | Diff
Patch (42.96 KB, patch)
2010-08-04 19:26 PDT, Matthew Delaney
no flags Details | Formatted Diff | Diff
Patch (43.44 KB, patch)
2010-08-05 11:48 PDT, Matthew Delaney
no flags Details | Formatted Diff | Diff
Patch (43.42 KB, patch)
2010-08-10 11:49 PDT, Matthew Delaney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description lvanbass+webkitbugzilla 2010-08-02 05:56:23 PDT
1) Open this page : http://codingwithattitude.net/gml/gml-drawer.html and draw lines on the black canvas (this uses regular mousedown, mousemove and mouseup events)

2) Click on the "clear" action.

Expected result:
The canvas should be reset by setting its width to the original width

Observed result:
Nothing. Subsequently drawing on the surface adds a black square covering the old drawing under the new drawing.

WebKit-r64451 nightly on Windows XP
Comment 1 Matthew Delaney 2010-08-03 21:11:53 PDT
Created attachment 63409 [details]
Comment 2 Darin Adler 2010-08-03 22:53:20 PDT
Comment on attachment 63409 [details]

Bug fixes need a test case. Could you make a test for this one please?
Comment 3 Darin Adler 2010-08-03 22:54:02 PDT
Comment on attachment 63409 [details]

review- due to lack of regression test

Fix itself looks good, although I might move the initialization of hadImageBuffer up even higher.
Comment 4 Matthew Delaney 2010-08-04 19:26:36 PDT
Created attachment 63534 [details]
Comment 5 Darin Adler 2010-08-05 07:35:10 PDT
Comment on attachment 63534 [details]

> -    bool ok;
> +    bool ok, hadImageBuffer = hasCreatedImageBuffer();

The two booleans should be defined on separate lines. We don't do two on one line like this in WebKit.

> Index: LayoutTests/fast/repaint/setWidthResetAfterForcedRender.html

While this does involve "repainting", I think this is a canvas test and should be in "fast/canvas".

Patch is OK as is, so review+ but I'm not setting commit-queue on this so you have a chance to decide whether to make some additional refinements before landing this.
Comment 6 Matthew Delaney 2010-08-05 11:48:32 PDT
Created attachment 63611 [details]
Comment 7 WebKit Commit Bot 2010-08-06 02:32:23 PDT
Comment on attachment 63611 [details]

Rejecting patch 63611 from commit-queue.

Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1
Last 500 characters of output:
	M	WebCore/html/HTMLCanvasElement.cpp
A repository hook failed: MERGE request failed on '/repository/webkit/trunk': Commit blocked by pre-commit hook (exit code 1) with output:

    The following files contain tab characters:


    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
 at /usr/local/git/libexec/git-core/git-svn line 572

Full output: http://queues.webkit.org/results/3576985
Comment 8 Matthew Delaney 2010-08-10 11:49:01 PDT
Created attachment 64035 [details]
Comment 9 Matthew Delaney 2010-08-10 12:18:52 PDT
Hopefully this one makes the commit bot happy :-)
Comment 10 Kenneth Rohde Christiansen 2010-08-14 08:05:41 PDT
Comment on attachment 64035 [details]

 +          hasCreatedImageBuffer up *before* the call to setSurface since setSurface
I guess this comment would be nice in the code as well.
Comment 11 WebKit Commit Bot 2010-08-14 08:37:06 PDT
Comment on attachment 64035 [details]

Clearing flags on attachment: 64035

Committed r65367: <http://trac.webkit.org/changeset/65367>
Comment 12 WebKit Commit Bot 2010-08-14 08:37:11 PDT
All reviewed patches have been landed.  Closing bug.