Bug 41392 - [V8] Move window.open into generic bindings
Summary: [V8] Move window.open into generic bindings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-29 23:42 PDT by Dominic Cooney
Modified: 2010-08-09 10:50 PDT (History)
2 users (show)

See Also:

Attachments
Moves window.open into the generic bindings. (30.22 KB, patch)
2010-07-01 21:49 PDT, Dominic Cooney 		no flags Details | Formatted Diff | Diff
Moves window.open into the generic bindings. (30.22 KB, patch)
2010-07-01 21:59 PDT, Dominic Cooney 		no flags Details | Formatted Diff | Diff
Patch (30.78 KB, patch)
2010-08-08 23:37 PDT, Dominic Cooney 		no flags Details | Formatted Diff | Diff
Patch (31.14 KB, patch)
2010-08-09 02:18 PDT, Dominic Cooney 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dominic Cooney 2010-06-29 23:42:56 PDT 
Like bug 33201 which moved createWindow into the generic bindings, logic for open can be moved into the generic bindings too.
Comment 1 Dominic Cooney 2010-07-01 21:49:34 PDT 
Created attachment 60331 [details]
Moves window.open into the generic bindings.
Comment 2 WebKit Review Bot 2010-07-01 21:53:29 PDT 
Attachment 60331 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/generic/BindingDOMWindow.h:167:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/bindings/generic/BindingDOMWindow.h:192:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
WebCore/bindings/v8/V8Utilities.cpp:41:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 3 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Dominic Cooney 2010-07-01 21:59:46 PDT 
Created attachment 60333 [details]
Moves window.open into the generic bindings.
Comment 4 Adam Barth 2010-07-02 13:53:11 PDT 
Comment on attachment 60333 [details]
Moves window.open into the generic bindings.

Sorry, need to run.  I got about half way through.

WebCore/bindings/generic/BindingDOMWindow.h:150
 +      Frame* enteredFrame = state->getFirstFrame();
Why not call this firstFrame?

WebCore/bindings/generic/BindingDOMWindow.h:154
 +      Frame* callingFrame = state->getActiveFrame();
Why not call this activeFrame?

WebCore/bindings/generic/BindingDOMWindow.h:193
 +              && (!protocolIsJavaScript(completedUrl) || ScriptController::isSafeScript(frame))) {
isSafeScript is a terrible name.  I don't think this is correct.  You need to call something on BindingsSecurity and pass in the state.

WebCore/bindings/generic/BindingSecurity.h:150
 +      Frame* callingFrame = state->getActiveFrame();
again, should be "activeFrame"
Comment 5 Dominic Cooney 2010-08-08 23:37:54 PDT 
Created attachment 63861 [details]
Patch
Comment 6 Dominic Cooney 2010-08-09 02:18:23 PDT 
Created attachment 63873 [details]
Patch
Comment 7 Adam Barth 2010-08-09 10:22:50 PDT 
Comment on attachment 63873 [details]
Patch

This looks great.  Now that we have more of this fleshed out, we should talk to some JSC folks and see if they want to use some of it.
Comment 8 Eric Seidel (no email) 2010-08-09 10:50:50 PDT 
Comment on attachment 63873 [details]
Patch

Clearing flags on attachment: 63873

Committed r64991: <http://trac.webkit.org/changeset/64991>
Comment 9 Eric Seidel (no email) 2010-08-09 10:50:55 PDT 
All reviewed patches have been landed.  Closing bug.