Bug 58442

Summary: video controls have no-longer-necessary bottom placement on media documents
Product: WebKit Reporter: Ami Fischman <fischman>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, jer.noble
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on:    
Bug Blocks: 54634    
Attachments:
Description Flags
Remove bogus bottom margin from media-document video controls.
none
Stop harshing historical mellow.
none
Archive of layout-test-results from eseidel-cq-sf
none
Update for single (!) failing layout test. none

Description Ami Fischman 2011-04-13 08:10:34 PDT
Video controls in media documents should look the same as ones in HTML files, namely overlap the video canvas.  Not only is that required for consistency, it's also necessary in order to reason about the height of the rendered <video> element, which can be useful in iframes, for example.
Comment 1 Ami Fischman 2011-04-13 08:11:55 PDT
Created attachment 89378 [details]
Remove bogus bottom margin from media-document video controls.
Comment 2 Jer Noble 2011-04-13 09:24:38 PDT
The bottom margin for full page media is there for a reason: so the controls do not overlap the video during playback.  As such, it's not "bogus" but a intentional design decision by Apple's HI department.  Any changes which affect the to the look-and-feel of the media controls will just get reverted.

Please find another way to achieve what you are trying to do here.
Comment 3 Jer Noble 2011-04-13 09:30:41 PDT
Ah, I've been told that I'm misinformed.  The original reason for putting the controls under the video was because previously controls couldn't be rendered over the video.  I actually fixed that bug recently, so there is no longer an impediment to rendering the controls over the video area.  HI will get used to it. :)
Comment 4 Jer Noble 2011-04-13 09:31:35 PDT
Still, the decision wasn't "bogus" but completely necessary at the time.
Comment 5 Jer Noble 2011-04-13 09:32:05 PDT
Anyway, objection withdrawn. :)
Comment 6 Eric Carlson 2011-04-13 09:39:47 PDT
Comment on attachment 89378 [details]
Remove bogus bottom margin from media-document video controls.

View in context: https://bugs.webkit.org/attachment.cgi?id=89378&action=review

r- only because I think it would be good to have a more accurate descriptions in the change log.

> Source/WebCore/ChangeLog:5
> +        Remove the bogus bottom margin of controls in video elements on media documents.

"bogus" is incorrect, how about "unnecessary" instead?
Comment 7 Ami Fischman 2011-04-13 09:43:36 PDT
Created attachment 89391 [details]
Stop harshing historical mellow.
Comment 8 Ami Fischman 2011-04-13 09:44:29 PDT
Amended ChangeLog and rephrased the bug summary to be more reflective of reality.
Sorry for my previous phrasing.
Comment 9 WebKit Commit Bot 2011-04-14 01:59:00 PDT
Comment on attachment 89391 [details]
Stop harshing historical mellow.

Rejecting attachment 89391 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'bu..." exit_code: 2

Last 500 characters of output:
ests/xmlhttprequest ................................................................................................................................................................................
http/tests/xmlhttprequest/web-apps ...............
http/tests/xmlhttprequest/workers ...........
http/tests/xmlviewer .
http/tests/xmlviewer/dumpAsText ............
754.05s total testing time

23268 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
16 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/8397557
Comment 10 WebKit Commit Bot 2011-04-14 01:59:02 PDT
Created attachment 89549 [details]
Archive of layout-test-results from eseidel-cq-sf

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: eseidel-cq-sf  Port: Mac  Platform: Mac OS X 10.6.4
Comment 11 Ami Fischman 2011-04-14 08:52:58 PDT
Created attachment 89588 [details]
Update for single (!) failing layout test.
Comment 12 WebKit Commit Bot 2011-04-14 20:43:56 PDT
Comment on attachment 89588 [details]
Update for single (!) failing layout test.

Clearing flags on attachment: 89588

Committed r83939: <http://trac.webkit.org/changeset/83939>
Comment 13 WebKit Commit Bot 2011-04-14 20:44:05 PDT
All reviewed patches have been landed.  Closing bug.