Bug 22070 - Add an option to allow scripts to close windows.
Summary: Add an option to allow scripts to close windows.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Zachary Kuznia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-11-04 15:25 PST by Zachary Kuznia
Modified: 2009-01-28 11:19 PST (History)
0 users

See Also:


Attachments
Patch to implement this feature (3.08 KB, patch)
2008-11-04 15:27 PST, Zachary Kuznia
eric: review-
Details | Formatted Diff | Diff
Patch to implement this feature (3.05 KB, patch)
2008-11-25 16:44 PST, Zachary Kuznia
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zachary Kuznia 2008-11-04 15:25:48 PST
This is a useful option for automated testing.  It allows you to set up test pages that clean up after themselves when they're done.
Comment 1 Zachary Kuznia 2008-11-04 15:27:10 PST
Created attachment 24900 [details]
Patch to implement this feature
Comment 2 Eric Seidel (no email) 2008-11-25 16:12:48 PST
Comment on attachment 24900 [details]
Patch to implement this feature

The patch looks fine in general, but the variable names don't follow WebKit's style guidelines:
http://webkit.org/coding/coding-style.html
Comment 3 Zachary Kuznia 2008-11-25 16:44:24 PST
Created attachment 25513 [details]
Patch to implement this feature

Fixed the variable names to be CamelCase
Comment 4 Sam Weinig 2008-11-25 19:44:02 PST
Comment on attachment 25513 [details]
Patch to implement this feature

> +    bool allowScriptsToCloseWindows =
> +        (settings && settings->allowScriptsToCloseWindows());
No reason to put these on two lines.

> @@ -232,6 +235,7 @@ namespace WebCore {
>          bool m_zoomsTextOnly : 1;
>          bool m_enforceCSSMIMETypeInStrictMode : 1;
>          size_t m_maximumDecodedImageSize;
> +        bool m_allowScriptsToCloseWindows : 1;
If you move this bool up one place, it will not cause unnessary bloating of the object.

This patch looks fine, but it is not hooked up to anything.  Usually when we expose a new setting, we hook it up to the WebPreferences API (or probably SPI in this case) in WebKit.  r- since this really won't do anything on its own.
Comment 5 Sam Weinig 2008-12-23 13:45:46 PST
Comment on attachment 25513 [details]
Patch to implement this feature

I am reversing my decision here.  It is a little odd to add a new setting without adding WebKit SPI to do something with it, and I would rather you did, but it is not strictly necessary.
Comment 6 Darin Fisher (:fishd, Google) 2009-01-28 11:19:50 PST
http://trac.webkit.org/changeset/40315