Bug 25066 - Video controls misplaced when dynamically adding reflection or opacity
Summary: Video controls misplaced when dynamically adding reflection or opacity
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-06 17:46 PDT by Simon Fraser (smfr)
Modified: 2009-04-18 22:01 PDT (History)
3 users (show)

See Also:


Attachments
Testcase: hover over the video (657 bytes, text/html)
2009-04-06 17:46 PDT, Simon Fraser (smfr)
no flags Details
Patch, testcase, changelog (8.92 KB, patch)
2009-04-17 22:41 PDT, Simon Fraser (smfr)
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2009-04-06 17:46:19 PDT
The video control bar shows up in the wrong location when you dynamically add a reflection, say on hover.
Comment 1 Simon Fraser (smfr) 2009-04-06 17:46:40 PDT
Created attachment 29297 [details]
Testcase: hover over the video
Comment 2 Simon Fraser (smfr) 2009-04-14 22:17:24 PDT
Same bug happens with opacity. Seems to be related to the video element getting a layer.
Comment 3 Simon Fraser (smfr) 2009-04-15 23:06:22 PDT
The problem here is that RenderMedia lies about the RenderObject hierarchy. It overrides RenderMedia::children() to return the children of the m_controlsShadowRoot renderer, thus skipping that renderer itself. This confuses RenderLayer::insertOnlyThisLayer(), which iterates through its child renderers, calling RenderObject::moveLayers() on each one. For RenderMedia, this finds the wrong renderers (which is important because the m_controlsShadowRoot has a layer!).

If we actually add the m_controlsShadowRoot renderer as a child of the RenderMedia, and don't like about the render hierarchy, it works fine.

Antti, was there a reason for faking the renderer hierarchy here?
Comment 4 Antti Koivisto 2009-04-16 18:35:30 PDT
Simon, I think you should ask Hyatt who did that change.
Comment 5 Antti Koivisto 2009-04-16 18:38:03 PDT
Probably just a mistake.
Comment 6 Simon Fraser (smfr) 2009-04-16 20:47:31 PDT
Those changes came from r28397:
http://trac.webkit.org/changeset/28397#file11
Comment 7 Antti Koivisto 2009-04-16 22:00:59 PDT
No they are not. They are from http://trac.webkit.org/changeset/40461.
Comment 8 Simon Fraser (smfr) 2009-04-16 23:27:57 PDT
Ah, right. Dave, can you explain what you did here?
http://trac.webkit.org/changeset/40461/trunk/WebCore/rendering/RenderMedia.cpp
Comment 9 Simon Fraser (smfr) 2009-04-17 22:41:08 PDT
Created attachment 29598 [details]
Patch, testcase, changelog
Comment 10 Antti Koivisto 2009-04-18 00:44:03 PDT
Comment on attachment 29598 [details]
Patch, testcase, changelog

r=me
Comment 11 Simon Fraser (smfr) 2009-04-18 22:01:08 PDT
http://trac.webkit.org/changeset/42651