[webkit-reviews] review denied: [Bug 24070] Changing "scrolling" attribute on iframe element already in DOM doesn't take effect : [Attachment 28060] First attempt to fix the bug

[bugzilla-daemon at webkit.org]

Cameron Zwarich (cpst) <cwzwarich at uwaterloo.ca> has denied Bo Yang
<techrazy.yang at gmail.com>'s request for review:
Bug 24070: Changing "scrolling" attribute on iframe element already in DOM
doesn't take effect
https://bugs.webkit.org/show_bug.cgi?id=24070

Attachment 28060: First attempt to fix the bug
https://bugs.webkit.org/attachment.cgi?id=28060&action=review

------- Additional Comments from Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>
+	 if (attached()) {

Is there any need to check attached() here? Shouldn't checking contentFrame()
be enough? 

+	     if (contentFrame() && contentFrame()->view()) {
+		 FrameView* view = contentFrame()->view();
+		 view->changeScrollbarsState(m_scrolling !=
ScrollbarAlwaysOff);
+	     }
+	 }

There is already a method on FrameView for setting the scrollbar state:

void setCanHaveScrollbars(bool canScroll);

You should use that instead of making your own. I'm r-'ing this patch, but I am
glad that you are fixing this bug.