Update behaviour as per http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#pixel-manipulation:
"The getImageData(sx, sy, sw, sh) method must return an ImageData object representing the underlying pixel data for the area of the canvas denoted by the rectangle whose corners are the four points (sx, sy), (sx+sw, sy), (sx+sw, sy+sh), (sx, sy+sh), in canvas coordinate space units."
Added block on master bug
Created attachment 71821 [details]
Added jamesr to cc list.
FWIW, this patch looks good to me. Though, does it end up making 2d.imageData.get.source.negative pass entirely on Mac? I note on my machine that the test fails 3 pixels of the imgdata1 and one of the imgdata2's pixels. Off the top of my head, I would guess that that imgdata2's pixel still fails.
I tested it on my Mac in Safari, and on Chromium Windows, and it passed entirely on both. run-webkit-tests also reported it passing correctly.
Running the test in a current Chrome, I get:
Failed assertion imgdata1.data["0"] === 255 (got 0[number], expected 255[number])
Failed assertion imgdata1.data["1"] === 255 (got 0[number], expected 255[number])
Failed assertion imgdata1.data["2"] === 255 (got 0[number], expected 255[number])
Failed assertion imgdata2.data["3"] === 0 (got 255[number], expected 0[number])
But running it in a patched Safari, I get it saying 'Passed', as per the expectations.
Ok great. Make sure then to remove the test from the LayoutTests/platform/mac/Skipped list and that it still passes from run-webkit-tests.
I had a patch out to unskip all the tests and reset the expectations, but it had some troubles with the bots and I need to resubmit. That should hopefully clear up future confusions about whether these tests are passing/failing and fixed or not.
Yep - the patch does remove the test from LayoutTests/platform/mac/Skipped (and also from chromium's test_expectations.txt, since I tested that platform too).
Adding Oliver to cc:
Oliver: could you take a look at this patch? Thanks!
Comment on attachment 71821 [details]
Comment on attachment 71821 [details]
Rejecting patch 71821 from commit-queue.
Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 71821]" exit_code: 1
Logging in as firstname.lastname@example.org...
Processing 1 patch from 1 bug.
Cleaning working directory
Updating working directory
Processing patch 71821 from bug 48277.
ERROR: /Projects/CommitQueue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Full output: http://queues.webkit.org/results/4746045
Created attachment 72402 [details]
Since I landed the patch to (finally) stop skipping all those (including this) canvas tests on mac, if this patch fixes the mac failing expectations then you'll need to update that file as well. It's located at LayoutTests/platform/mac-snowleopard/canvas/philip…
The mac-leopard skipped list currently contains this test so there's no need to update those expectations. Though, if this certainly fixes the problem on both snowleopard and leopard (likely the case) you can just get the expectation (of passing) file into LayoutTests/platform/mac/canvas/philip… in which case it'll apple to both leopard and snowleopard.
The patch does remove the snowleopard expectations file, since I can confirm that the test passes on snowleopard. I can't see why it wouldn't work on leopard, but since I didn't test it, I haven't touched those.
Are you on IRC? I can't find you. I can explain the Skipped lists in more detail.
Basically, if you have a fix (presumably) for all the ports, you'll just want to reset the project-wide expectations file (to now be passing) and then get rid of each port's expectation file if they have one. That way none of the test bots for those ports will show red on that test if the patch did indeed fix the bug.
Comment on attachment 72402 [details]
Clearing flags on attachment: 72402
Committed r70957: <http://trac.webkit.org/changeset/70957>
All reviewed patches have been landed. Closing bug.
Followup patch to remove entries in other ports Skipped files: https://bugs.webkit.org/show_bug.cgi?id=48702