Bug 27798

Summary: [chromium] Media control panel for <video> in MediaDocument is mis-placed
Product: WebKit Reporter: Hin-Chung Lam <hclam>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: fischman, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
overlap
none
patch
levin: review+
Patch levin: review-

Description Hin-Chung Lam 2009-07-29 02:22:11 PDT
Created attachment 33703 [details]
overlap

When a video file is loaded by MediaDocument, <video> is used to play the video file, the control panel is misplaced and overlaps with the video by 16 pixels.
Comment 1 Hin-Chung Lam 2009-07-29 02:31:48 PDT
Created attachment 33706 [details]
patch
Comment 2 David Levin 2009-07-29 02:50:05 PDT
Comment on attachment 33706 [details]
patch

> Index: WebCore/ChangeLog

> +2009-07-29  Alpha Lam  <hclam@chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).

This should be added to the changelog:

  Media control panel for <video> in MediaDocument is mis-placed
  https://bugs.webkit.org/show_bug.cgi?id=27798

If you can use prepare-ChangeLog --bug #, this will be set-up for you.
Comment 3 David Levin 2009-07-29 02:53:25 PDT
Committed as http://trac.webkit.org/changeset/46534
Comment 4 Ami Fischman 2011-02-14 14:24:51 PST
Created attachment 82363 [details]
Patch
Comment 5 Ami Fischman 2011-02-14 14:26:23 PST
Motivation for the patch I just attached is the linked chromium bug on test_shell.  Hopefully merely commenting here will reopen this bug.  If using a new bug is preferred please let me know.
Comment 6 David Levin 2011-02-14 14:37:20 PST
Comment on attachment 82363 [details]
Patch

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

A new bug is preferred for new work/patches -- otherwise it can be confusing to sort through the comments (which pertain to the current patch and which pertain to the old patch).

> Source/WebCore/ChangeLog:13
> +        No new tests. (OOPS!)

This line needs to be replaced :)

A few choices:
1. Indicate what test you're adding.
2. Indicate what test covers the functionality.
3. Indicate why a test isn't necessary (No new functionality, so no new tests.) (Hint that isn't the case here because you're changing something that is visible.)
4. Indicate why a test isn't possible (You should add functionality to dumprendertree if necessary to test, so this is almost never the correct answer.)
Comment 7 Ami Fischman 2011-02-14 22:41:20 PST
@levin: thanks for the info & comments.  I opened bug 54436 and attached an updated patch which contains a layout test that asserts the new behavior.