[webkit-reviews] review denied: [Bug 26659] Support hidding an control bar element from the Media element controller. : [Attachment 31745] patch v2.

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Pierre d'Herbemont
<pdherbemont at apple.com>'s request for review:
Bug 26659: Support hidding an control bar element from the Media element
controller.
https://bugs.webkit.org/show_bug.cgi?id=26659

Attachment 31745: patch v2.
https://bugs.webkit.org/attachment.cgi?id=31745&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
You ChangeLog is missing the bug url.

Style:
+    if(m_isHidden)
+    if(!m_mediaElement || !m_mediaElement->renderer())
+    if (!renderer() && !m_isHidden && style->display() != NONE)
+    {
+	 if (renderer)
+	 {

When does this ever return NULL?
+	 RenderObject* renderer =
createRenderer(m_mediaElement->renderer()->renderArena(), style);

Why don't we just early-return in that case?

Why manually set attached instead of calling attach?
+	     setAttached();