Currently, strokeRect will stroke a rectangle whose dimensions are 0 when lineWidth > 1. As per the definition of strokeRect in the HTML Canvas 2D Context spec. <http://www.w3.org/TR/2dcontext/#dom-context-2d-strokerect>, this method should have no effect when both the height and width of the rectangle are zero.
Created attachment 53506 [details] Patch with test case
Comment on attachment 53506 [details] Patch with test case I think it would be better to make validateRectForCanvas do the check as that gets the validation in a single place that is used uniformly -- is there any place this might have a bad impact?
(In reply to comment #2) > (From update of attachment 53506 [details]) > I think it would be better to make validateRectForCanvas do the check as that > gets the validation in a single place that is used uniformly -- is there any > place this might have a bad impact? I agree, it is better to place this check in validateRectForCanvas. There should not be any negative impact as this is an invalid state for clearRect, and fillRect as well.
Created attachment 53520 [details] Patch with test case
Comment on attachment 53520 [details] Patch with test case Will add some more test cases.
Created attachment 53524 [details] Patch with test cases Added additional test cases for fillRect, clearRect, and rect.
I think the change and the tests are ok. What do you think Oliver?
Comment on attachment 53524 [details] Patch with test cases Daniel can you just verify behaviour if you have a shadow set? eg. should the shadow draw around a 0x0 rect (i know physically it shouldn't i just want to verify firefox compat) If the shadow behaviour is consistent the patch is good --Oliver
Created attachment 54255 [details] Example strokeRect of 0x0 rect with lineWidth > 1 and shadow (In reply to comment #8) > (From update of attachment 53524 [details]) > Daniel can you just verify behaviour if you have a shadow set? eg. should the > shadow draw around a 0x0 rect (i know physically it shouldn't i just want to > verify firefox compat) Oliver, Firefox (3.6.3) does not draw a shadow around a 0x0 rect. I've attached one such test case. Dan > > If the shadow behaviour is consistent the patch is good > > --Oliver
Created attachment 54256 [details] Patch with test cases Added a test case for strokeRect of 0x0 rect with lineWidth > 1 and shadow. Also, rebased results LayoutTests/fast/canvas/canvas-strokeRect-expected.txt.
Comment on attachment 54256 [details] Patch with test cases r=me
Comment on attachment 54256 [details] Patch with test cases Clearing flags on attachment: 54256 Committed r58381: <http://trac.webkit.org/changeset/58381>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/58381 might have broken Qt Linux Release
The failure looks real: http://build.webkit.org/results/Qt%20Linux%20Release/r58381%20(10853)/fast/repaint/selection-gap-overflow-scroll-2-diffs.txt Not sure if it's related to this bug or not.
(In reply to comment #14) > http://trac.webkit.org/changeset/58381 might have broken Qt Linux Release The failing test, /fast/repaint/selection-gap-overflow-scroll-2.html, is unrelated to this change. Looking at the differences <http://build.webkit.org/results/Qt%20Linux%20Release/r58381%20(10853)/fast/repaint/selection-gap-overflow-scroll-2-pretty-diff.html> they seem to differ by 1-pixel. For completeness, the stdio from the Qt bot is at: <http://build.webkit.org/builders/Qt%20Linux%20Release/builds/10853/steps/layout-test/logs/stdio>
(In reply to comment #16) I made a clean build, and nothing changed. This test needs only a rebaseline.
(In reply to comment #17) > (In reply to comment #16) > I made a clean build, and nothing changed. This test needs only a rebaseline. Committed rebased result in change set 58388 <http://trac.webkit.org/changeset/58388>.