Summary: | Layout Test (media/video-zoom.html) fails on Mac (crbug.com/70252) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ami Fischman <fischman> | ||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, commit-queue, eric, mihaip, sergeyu, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Ami Fischman
2011-02-03 16:56:12 PST
Created attachment 81145 [details]
Patch
Reviewer: please flip commit-queue+ bit when ready. (I'm a chromium/webkit noob and didn't realize I should pass --request-commit to webkit-patch upload) Comment on attachment 81145 [details]
Patch
Your ChangeLog should explain why. Is this no longer flaky? Why are these rebaselines differnet from other ports?
Created attachment 81382 [details]
Patch
Thanks for the quick review Eric. I'm new here so I appreciate you making expectations/process explicit. I expanded the ChangeLog entry in the updated patch. I only changed the one port's baseline b/c that's what ./webkit/tools/layout_tests/run_webkit_tests.sh --new-baseline --build-directory=`pwd`/ninja media/video-zoom.html thought was necessary. If that's not what you were getting at with "Why are these rebaselines differnet from other ports?", can you be more explicit about what you're looking for? So these are all the scrollbar drawing bug? (I'm pretty sure Mihai filed a Radar with Apple about such.) > So these are all the scrollbar drawing bug?
image_diff says nothing changed other than the scrollbar, yes.
Are you saying this patch shouldn't be committed b/c the checked-in expectation is actually correct and the scrollbar showing up differently is a reported bug?
(In reply to comment #7) > > So these are all the scrollbar drawing bug? > > image_diff says nothing changed other than the scrollbar, yes. > Are you saying this patch shouldn't be committed b/c the checked-in expectation is actually correct and the scrollbar showing up differently is a reported bug? This test has chromium-specific baselines since (apparently) we render scaled media elements differently from the mac port. When I landed my scrollbar change to make chromium-mac consistent with mac (http://trac.webkit.org/changeset/74892), I had to rebaseline such tests (since they had chromium-mac baselines checked in with the old chromium-specific scrollbar drawing). However, I missed this one since it was marked as failing at that time (for flakyness reasons which are apparently no longer true). (In reply to comment #7) > Are you saying this patch shouldn't be committed b/c the checked-in expectation is actually correct and the scrollbar showing up differently is a reported bug? Just to be clear, the scrollbar change is for the better, in that the chromium-mac scrollbars now match the mac ones. If you delete the video-zoom-expected.png/checksum files in chromium-mac and run this test with the mac pixel baselines, you'll see that the the only diffs are in the media element itself. Thanks for the info Mihai. I guess it's back to you, Eric, for review. Comment on attachment 81382 [details]
Patch
rs=me.
Comment on attachment 81382 [details] Patch Clearing flags on attachment: 81382 Committed r77897: <http://trac.webkit.org/changeset/77897> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/77897 might have broken GTK Linux 32-bit Release FTR: Build breakage was unrelated; GTK Linux 32-bit Release build was unbroken by http://build.webkit.org/changes/27435 |