Bug 24070 – Changing "scrolling" attribute on iframe element already in DOM doesn't take effect

• Status: RESOLVED
• Resolution: FIXED
• Component: Scrolling

[Tom Robinson]

Changing "scrolling" attribute on iframe element already in DOM doesn't take effect.

Works as expected in Firefox.

[Tom Robinson]

Created attachment 27847 [details]
Reduction

[Bo Yang]

conform on Windows Safari.

[Bo Yang]

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.

[Cameron Zwarich (cpst)]

*** Bug 24972 has been marked as a duplicate of this bug. ***

[Cameron Zwarich (cpst)]

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.

[Ahmad Saleem]

I am able to reproduce this in Safari 15.6 on macOS using modified test case by changing HTTP to HTTPS in test case:

Link - https://jsfiddle.net/fcjd6qr8/

Following are results across browsers:

*** Firefox Nightly 105 ***

Once iframe load, clicking "show scrollbars" load "scrollbar" in iframe

*** Chrome Canary 106 ***

Nothing loads and "show scrollbars" do nothing.

*** Safari 15.6 on macOS 12.5 ***

Nothing loads and "show scrollbars" do nothing.

________

I am not sure on web-spec but just wanted to share updated testing results. Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/98911472>

[Simon Fraser (smfr)]

Created attachment 461752 [details]
Reduction

[Simon Fraser (smfr)]

HTMLFrameElementBase::parseAttribute() needs to respond to "scrolling" attribute changes.

[Matt Woodrow]

*** This bug has been marked as a duplicate of bug 281103 ***

[Matt Woodrow]

I'll fix this as a separate task.

[Matt Woodrow]

Pull request: https://github.com/WebKit/WebKit/pull/34933

[EWS]

Committed 284947@main (d98621968e3c): <https://commits.webkit.org/284947@main>

Reviewed commits have been landed. Closing PR #34933 and removing active labels.