[webkit-reviews] review denied: [Bug 41392] [V8] Move window.open into generic bindings : [Attachment 60333] Moves window.open into the generic bindings.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 2 13:53:11 PDT 2010


Adam Barth <abarth at webkit.org> has denied Dominic Cooney
<dominicc at google.com>'s request for review:
Bug 41392: [V8] Move window.open into generic bindings
https://bugs.webkit.org/show_bug.cgi?id=41392

Attachment 60333: Moves window.open into the generic bindings.
https://bugs.webkit.org/attachment.cgi?id=60333&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
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"


More information about the webkit-reviews mailing list