[webkit-reviews] review denied: [Bug 14845] Frame's noResize attribute can not be set by JavaScript : [Attachment 27859] A better version

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 11 05:52:43 PDT 2009


Eric Seidel <eric at webkit.org> has denied Bo Yang <techrazy.yang at gmail.com>'s
request for review:
Bug 14845: Frame's noResize attribute can not be set by JavaScript
https://bugs.webkit.org/show_bug.cgi?id=14845

Attachment 27859: A better version
https://bugs.webkit.org/attachment.cgi?id=27859&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
 130	     if (attached() && parentNode()->renderer()->isFrameSet()) {
 131		 HTMLFrameSetElement* fs =
static_cast<HTMLFrameSetElement*>(parentNode());

seems like an unsafe check.  Just because the renderer() is a frameset, doesn't
necessarily mean the Element is.  I think you should check hasTagName instead.

I've never seen this convention before:
m_noResize = !attr->isNull();

I didn't realize that parseMappedAttribute was called when an attribute was
removed?

I also don't see how attr->isNull() will return true when noResize="false"?

The test case would be better if it printed PASS or FAIL instead of just
printing the resulting metrics.

r- for the attr->isNull() issue.  I'm confused as to if that code actually
works.


More information about the webkit-reviews mailing list