[Webkit-unassigned] [Bug 54634] media/video-controls-in-media-document.html has image+text diffs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54634





--- Comment #26 from Ami Fischman <fischman at chromium.org>  2011-04-11 13:34:25 PST ---
(In reply to comment #24)
> This doesn't hard code the control height, so why use "240" for the video height? Maybe I am being pedantic because we *do* know the video dimensions, but is there any reason to not calculate the height?

My motivation is that I was worried that video.offsetHeight might end up including the height of the controls and I wanted to avoid having to worry about finding the canvas part of the element, so chose readability.  But it seems like that worry was unfounded, so changed.

> Also I think it is good to structure a "test(...)" so the output shows what is being tested instead of just the result. 

Agreed.

> For example:
>     testExpected("controls.offsetTop", video.offsetHeight, "<");
> will generate: 
>     EXPECTED (video.offsetTop < '240') OK

In fact it generates: ReferenceError: controls is not defined :)
I didn't think about it too much before and just changed to test(), but I can regain the kind of message you want by dropping the "var" to bleed |controls| out of function scope.  Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.