Bug 25066

Summary: Video controls misplaced when dynamically adding reflection or opacity
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, hyatt, koivisto
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Testcase: hover over the video
none
Patch, testcase, changelog koivisto: review+

Simon Fraser (smfr)
Reported 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.
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+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2009-04-06 17:46:40 PDT
Created attachment 29297 [details] Testcase: hover over the video
Simon Fraser (smfr)
Comment 2 2009-04-14 22:17:24 PDT
Same bug happens with opacity. Seems to be related to the video element getting a layer.
Simon Fraser (smfr)
Comment 3 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?
Antti Koivisto
Comment 4 2009-04-16 18:35:30 PDT
Simon, I think you should ask Hyatt who did that change.
Antti Koivisto
Comment 5 2009-04-16 18:38:03 PDT
Probably just a mistake.
Simon Fraser (smfr)
Comment 6 2009-04-16 20:47:31 PDT
Those changes came from r28397: http://trac.webkit.org/changeset/28397#file11
Antti Koivisto
Comment 7 2009-04-16 22:00:59 PDT
No they are not. They are from http://trac.webkit.org/changeset/40461.
Simon Fraser (smfr)
Comment 8 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
Simon Fraser (smfr)
Comment 9 2009-04-17 22:41:08 PDT
Created attachment 29598 [details] Patch, testcase, changelog
Antti Koivisto
Comment 10 2009-04-18 00:44:03 PDT
Comment on attachment 29598 [details] Patch, testcase, changelog r=me
Simon Fraser (smfr)
Comment 11 2009-04-18 22:01:08 PDT
http://trac.webkit.org/changeset/42651
Note You need to log in before you can comment on or make changes to this bug.