[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