Bug 24070 - Changing "scrolling" attribute on iframe element already in DOM doesn't take effect
Summary: Changing "scrolling" attribute on iframe element already in DOM doesn't take ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 24972 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-02-20 19:47 PST by Tom Robinson
Modified: 2010-06-10 16:55 PDT (History)
3 users (show)

See Also:


Attachments
Reduction (185 bytes, text/html)
2009-02-20 19:48 PST, Tom Robinson
no flags Details
First attempt to fix the bug (5.67 KB, patch)
2009-02-26 18:36 PST, Bo Yang
zwarich: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Robinson 2009-02-20 19:47:47 PST
Changing "scrolling" attribute on iframe element already in DOM doesn't take effect.

Works as expected in Firefox.
Comment 1 Tom Robinson 2009-02-20 19:48:21 PST
Created attachment 27847 [details]
Reduction
Comment 2 Bo Yang 2009-02-20 20:51:40 PST
conform on Windows Safari.
Comment 3 Bo Yang 2009-02-26 18:36:05 PST
Created attachment 28060 [details]
First attempt to fix the bug

Add a new method in FrameView to change the scrollbar's state and a test case for this bug.
Comment 4 Cameron Zwarich (cpst) 2009-03-31 18:22:35 PDT
*** Bug 24972 has been marked as a duplicate of this bug. ***
Comment 5 Cameron Zwarich (cpst) 2009-03-31 18:42:48 PDT
Comment on attachment 28060 [details]
First attempt to fix the bug

+        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.