Bug 53737 - Layout Test (media/video-zoom.html) fails on Mac (crbug.com/70252)
Summary: Layout Test (media/video-zoom.html) fails on Mac (crbug.com/70252)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-03 16:56 PST by Ami Fischman
Modified: 2011-02-08 07:38 PST (History)
6 users (show)

See Also:


Attachments
Patch (363.49 KB, patch)
2011-02-03 16:57 PST, Ami Fischman
no flags Details | Formatted Diff | Diff
Patch (364.03 KB, patch)
2011-02-05 16:53 PST, Ami Fischman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ami Fischman 2011-02-03 16:56:12 PST
Layout Test (media/video-zoom.html) fails on Mac (crbug.com/70252)
Comment 1 Ami Fischman 2011-02-03 16:57:38 PST
Created attachment 81145 [details]
Patch
Comment 2 Ami Fischman 2011-02-04 15:17:25 PST
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 3 Eric Seidel (no email) 2011-02-05 02:10:16 PST
Comment on attachment 81145 [details]
Patch

Your ChangeLog should explain why.  Is this no longer flaky?  Why are these rebaselines differnet from other ports?
Comment 4 Ami Fischman 2011-02-05 16:53:27 PST
Created attachment 81382 [details]
Patch
Comment 5 Ami Fischman 2011-02-05 16:54:26 PST
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?
Comment 6 Eric Seidel (no email) 2011-02-07 16:33:33 PST
So these are all the scrollbar drawing bug?  (I'm pretty sure Mihai filed a Radar with Apple about such.)
Comment 7 Ami Fischman 2011-02-07 16:39:44 PST
> 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?
Comment 8 Mihai Parparita 2011-02-07 16:49:24 PST
(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).
Comment 9 Mihai Parparita 2011-02-07 16:50:56 PST
(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.
Comment 10 Ami Fischman 2011-02-07 20:52:54 PST
Thanks for the info Mihai.
I guess it's back to you, Eric, for review.
Comment 11 Eric Seidel (no email) 2011-02-07 21:26:28 PST
Comment on attachment 81382 [details]
Patch

rs=me.
Comment 12 WebKit Commit Bot 2011-02-07 22:08:45 PST
Comment on attachment 81382 [details]
Patch

Clearing flags on attachment: 81382

Committed r77897: <http://trac.webkit.org/changeset/77897>
Comment 13 WebKit Commit Bot 2011-02-07 22:08:52 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 WebKit Review Bot 2011-02-07 22:32:28 PST
http://trac.webkit.org/changeset/77897 might have broken GTK Linux 32-bit Release
Comment 15 Ami Fischman 2011-02-08 07:38:26 PST
FTR: Build breakage was unrelated; GTK Linux 32-bit Release build was unbroken by http://build.webkit.org/changes/27435