WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
17811
Widget class refactoring
https://bugs.webkit.org/show_bug.cgi?id=17811
Summary
Widget class refactoring
Rodney Dawes
Reported
2008-03-12 14:52:18 PDT
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.
Attachments
Patch to refactor the Widget class
(42.81 KB, patch)
2008-03-12 14:53 PDT
,
Rodney Dawes
no flags
Details
Formatted Diff
Diff
Updated patch to fix tabs vs. spaces
(42.83 KB, patch)
2008-03-13 07:01 PDT
,
Rodney Dawes
darin
: review-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Rodney Dawes
Comment 1
2008-03-12 14:53:11 PDT
Created
attachment 19717
[details]
Patch to refactor the Widget class
Rodney Dawes
Comment 2
2008-03-13 07:01:53 PDT
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 3
2008-03-13 09:56:10 PDT
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)
Comment 4
2012-11-02 14:22:49 PDT
I suspect this patch/bug is obsolete by now and can be closed?
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug