Bug 17811 – Widget class refactoring

• Component: Platform
• Status: NEW

[Rodney Dawes]

Currently, the Widget class is split up with several #if PLATFORM() statements, with duplicate code across many platforms, as well as a circular dependency on the derived class ScrollView. This patch is intended to clean up the circular dependence and provide shared methods for the many duplicate methods, rather than copying the code between platforms.

[Rodney Dawes]

Created attachment 19717 [details]
Patch to refactor the Widget class

[Rodney Dawes]

Created attachment 19724 [details]
Updated patch to fix tabs vs. spaces

Oops. Forgot to fix the issues where tabs came up instead of spaces last night when submitting the patch. This revision fixes that.

[Darin Adler]

Comment on attachment 19724 [details]
Updated patch to fix tabs vs. spaces

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.

[Eric Seidel (no email)]

I suspect this patch/bug is obsolete by now and can be closed?