RESOLVED FIXED 16385
Cleanup kjs_window 
https://bugs.webkit.org/show_bug.cgi?id=16385
Summary Cleanup kjs_window
Sam Weinig
Reported 2007-12-10 13:52:12 PST
Most of the content of kjs_window can be move to more appropriate places (to DOMWindow, WindowFeatures, etc.)
Attachments
First round of cleanup (48.33 KB, patch)
2007-12-10 14:13 PST, Sam Weinig 		darin: review+
DetailsFormatted DiffDiff
move PausedTimeouts (145.58 KB, patch)
2007-12-16 13:40 PST, Sam Weinig 		darin: review+
DetailsFormatted DiffDiff
move ScheduledAction (146.77 KB, patch)
2007-12-16 16:48 PST, Sam Weinig 		mjs: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Sam Weinig
Comment 1 2007-12-10 13:54:13 PST
*** Bug 16386 has been marked as a duplicate of this bug. ***
Sam Weinig
Comment 2 2007-12-10 14:13:14 PST
Created attachment 17826 [details] First round of cleanup
Darin Adler
Comment 3 2007-12-10 14:20:56 PST
Comment on attachment 17826 [details] First round of cleanup Please run the sort-Xcode-project-file script on the project file. + // - dialogHide: trusted && WindowFeatures::boolFeature(features, "dialoghide"), makes dialog hide when you print + // - help: WindowFeatures::boolFeature(features, "help", true), makes help icon appear in dialog (what does it do on Windows?) + // - unadorned: trusted && WindowFeatures::boolFeature(features, "unadorned"); Seems a little strange here. Global replace problem? +static bool isSeparator(::UChar c) I don't think you need the :: here. It was needed because of "using namespace KJS". + {} We usually put those braces on separate lines. Where's the update to the .pro and .bkl files? Otherwise looks fine.
Sam Weinig
Comment 4 2007-12-16 13:40:50 PST
Created attachment 17937 [details] move PausedTimeouts
Sam Weinig
Comment 5 2007-12-16 14:52:35 PST
The first patch was landed in r28592.
Darin Adler
Comment 6 2007-12-16 15:36:30 PST
Comment on attachment 17937 [details] move PausedTimeouts r=me
Sam Weinig
Comment 7 2007-12-16 16:30:41 PST
Second patch landed in r28779.
Sam Weinig
Comment 8 2007-12-16 16:48:11 PST
Created attachment 17943 [details] move ScheduledAction
Maciej Stachowiak
Comment 9 2007-12-16 19:26:52 PST
Comment on attachment 17943 [details] move ScheduledAction Please try to use separate bugs for separate patches. Otherwise the state of the bug gets confusing. r=me
Sam Weinig
Comment 10 2007-12-16 20:52:10 PST
Third patch landed in r28794. Closing this bug, but will follow up with other bugs for more specific single tasks.
Note You need to log in before you can comment on or make changes to this bug.