Summary: | [V8] Move window.open into generic bindings | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dominic Cooney <dominicc> | ||||||||||
Component: | WebCore JavaScript | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | eric, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Dominic Cooney
2010-06-29 23:42:56 PDT
Created attachment 60331 [details]
Moves window.open into the generic bindings.
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.
Created attachment 60333 [details]
Moves window.open into the generic bindings.
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"
Created attachment 63861 [details]
Patch
Created attachment 63873 [details]
Patch
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 on attachment 63873 [details] Patch Clearing flags on attachment: 63873 Committed r64991: <http://trac.webkit.org/changeset/64991> All reviewed patches have been landed. Closing bug. |