[webkit-reviews] review denied: [Bug 53583] Refactoring: <progress> should not use ShadowElement : [Attachment 80908] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied MORITA Hajime
<morrita at google.com>'s request for review:
Bug 53583: Refactoring: <progress> should not use ShadowElement
https://bugs.webkit.org/show_bug.cgi?id=53583

Attachment 80908: Patch
https://bugs.webkit.org/attachment.cgi?id=80908&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80908&action=review

Can you explain a bit about the originalVisibility? I may have known but
forgotten, and now confuses me. I am easily confused. Why do we need this?

> Source/WebCore/html/HTMLProgressElement.cpp:40
> +class ProgressBarValueElement : public HTMLDivElement {

This class should live in html/shadow/ProgressBarValuePartElement

> Source/WebCore/html/HTMLProgressElement.cpp:53
> +#if 0

You didn't mean to leave this in :)

> Source/WebCore/rendering/RenderIndicator.h:37
> +    virtual bool shouldHidden() = 0;

shouldBeHidden()?

> Source/WebCore/rendering/RenderProgress.cpp:102
> +    for (RenderObject* child = firstChild(); child; child =
child->nextSibling())

Why is this here? Why can't we just use RenderProgress:layout()?