[webkit-reviews] review denied: [Bug 17811] Widget class refactoring : [Attachment 19724] Updated patch to fix tabs vs. spaces

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Rodney Dawes
<dobey at wayofthemonkey.com>'s request for review:
Bug 17811: Widget class refactoring
http://bugs.webkit.org/show_bug.cgi?id=17811

Attachment 19724: Updated patch to fix tabs vs. spaces
http://bugs.webkit.org/attachment.cgi?id=19724&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
This won't compile on Mac. There are two implementations of
Widget::frameGeometry.

Having a m_frameRect is wrong for PlatformScrollBarQt and for Mac widgets. And
having that field there, but unused, on those platforms is a recipe for
disaster.

We need to find a different way to share this code.

I'm really sad to see so much use of virtual functions. Our normal
cross-platform strategy is to use compile time #if, not virtual functions.

Changing setParent to take a Widget* is a bad idea. That's going in the wrong
direction. The only kind of parent we support is a FrameView*. We don't need a
general purpose "make any widget a child of any other widget" call.

I didn't get a chance to review this whole thing, but I think we need to do
this refactoring in smaller pieces rather than all at once, and keep all the
platforms working.