[webkit-reviews] review denied: [Bug 27721] [WML] Deck access control is completly broken : [Attachment 33943] Initial patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 6 20:47:14 PDT 2009


Eric Seidel <eric at webkit.org> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 27721: [WML] Deck access control is completly broken
https://bugs.webkit.org/show_bug.cgi?id=27721

Attachment 33943: Initial patch
https://bugs.webkit.org/attachment.cgi?id=33943&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Does WML execute script?  Is it possible to change these values?  If so,
setting to an invalid value will not clear domain or path since you if
(value.isEmpty()) return before setting.  Is that intentional behavior?

What does this do, and why?
    if (!initialized) {
 58	    if (WMLPageState* wmlPageState = wmlPageStateForDocument(this)) {
 59		if (Page* page = wmlPageState->page())
 60		    page->goBack();
 61	    }
 62	}
Why not use early return there?

typo:
 71	// Remember that we'e successfully entered the deck

Seems:
bool initialized
is the wrong name.  It's accessAllowed or something like that.

Change seems large.  Too large for my little brain at this moment.  maybe we
could/should add the layout tests first...

r- hoping that we could split this to make it easier to review.


More information about the webkit-reviews mailing list