Bug 16385

Summary:

Cleanup kjs_window

Product:

WebKit

Reporter:

Sam Weinig <sam>

Component:

WebCore JavaScript

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
First round of cleanup	darin: review+
move PausedTimeouts	darin: review+
move ScheduledAction	mjs: review+

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+	Details Formatted Diff Diff
move PausedTimeouts (145.58 KB, patch) 2007-12-16 13:40 PST, Sam Weinig	darin: review+	Details Formatted Diff Diff
move ScheduledAction (146.77 KB, patch) 2007-12-16 16:48 PST, Sam Weinig	mjs: review+	Details Formatted Diff Diff
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.