[iOS] IFrames are not scrollable even when scrolling=yes is specified

• Component: Frames
• Status: RESOLVED CONFIGURATION CHANGED

[Dima Voytenko]

This bin reproduces the issue: http://jsbin.com/dedega

<iframe scrolling=yes> has no effect on iOS Safari (tested against iOS 9) - the iframe is not scrollable. This is supposed to work according to spec and works virtually on every browser/device I tried except iOS Safari, including desktop Safari.

Please advise.

[Rick Byers]

Tested on iOS 9, still broken.

It looks like web developers have struggled with this for awhile, and claim it got worse in iOS 8 eg:
http://stackoverflow.com/questions/26046373/iframe-scrolling-ios-8
https://css-tricks.com/forums/topic/scrolling-iframe-on-ipad/

Simon, any chance your work on improving viewports/scrolling might help here?  It seems really confusing that an iframe should behave differently from a scrollable div in this regard, and that iOS would be different from desktop Safari.

Dima tells me that the obvious work-around of putting a scrollable div inside the iframe is problematic for him because he doesn't have control of the iframe content/styling.  He can make the iframe body overflow:auto (along with html) and that works, but then is burned by bug 106133 (can't get/set the scroll position because body.scrollTop actually refers to the viewport).  Ultimately he ends up having to use a dummy DIV and getBoundlingClientRect in order to detect the scroll position of the iframe, yuck!

[Simon Fraser (smfr)]

iOS has always "flattened" (expanded) iframes. This was done to avoid the user getting trapped in a hierarchy of scrollable regions, which was thought to give a bad user experience on a small screen.

It may be time to reconsider this approach.

[zalan]

(In reply to comment #2)
> iOS has always "flattened" (expanded) iframes. This was done to avoid the
> user getting trapped in a hierarchy of scrollable regions, which was thought
> to give a bad user experience on a small screen.
> 
> It may be time to reconsider this approach.
I second that!

[Dima Voytenko]

Second it as well. I can see how this would be a problem with legacy sites overusing some of these tools like IFrames. But I think that time is gone or at least developers should be able to override this.

[Simon Fraser (smfr)]

Ojan suggested not flattening frames on pages with viewport tags.

[David Kilzer (:ddkilzer)]

<rdar://problem/22961085>

[Ries]

Don't know if I am supposed to here, but I'd like to upvote this issue. It's really a pain in 2016 that on iOS only the iframe changes its size, while it does not on other platforms.

[Frédéric Wang (:fredw)]

Created attachment 307586 [details]
testcase

[Frédéric Wang (:fredw)]

Created attachment 308506 [details]
Do not flatten iframes

This is a preliminary patch to remove flattening for iframe. However, it's not enough to make scrolling work (I'm still investigating this...).

In particular, we have an issue similar to bug 163911. A quick workaround to make scrolling work with the Find operation is:

--- a/Source/WebCore/editing/Editor.cpp
+++ b/Source/WebCore/editing/Editor.cpp
@@ -3122,2 +3122,2 @@ bool Editor::findString(const String& target, FindOptions options)
-    if (!(options & DoNotRevealSelection))
-        m_frame.selection().revealSelection();
+    // if (!(options & DoNotRevealSelection))
+    m_frame.selection().revealSelection();
--- a/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
+++ b/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
@@ -1370,3 +1370,4 @@ void WebFrameLoaderClient::transitionToCommittedForNewPage()
 #if PLATFORM(IOS)
-    m_frame->coreFrame()->view()->setDelegatesScrolling(true);
+    if (m_frame->coreFrame()->isMainFrame())
+        m_frame->coreFrame()->view()->setDelegatesScrolling(true);
 #endif

[Simon Fraser (smfr)]

Turning off frame flattening on iOS is a big usability change and needs to be done with caution.

[zalan]

(In reply to Simon Fraser (smfr) from comment #10)
> Turning off frame flattening on iOS is a big usability change and needs to
> be done with caution.
and I don't see the point of doing it for iframes only.

[Frédéric Wang (:fredw)]

(In reply to Simon Fraser (smfr) from comment #10)
> Turning off frame flattening on iOS is a big usability change and needs to
> be done with caution.

(In reply to zalan from comment #11)
> (In reply to Simon Fraser (smfr) from comment #10)
> > Turning off frame flattening on iOS is a big usability change and needs to
> > be done with caution.
> and I don't see the point of doing it for iframes only.

@Simon, @Zalan: This was really an experimental patch to avoid flattening and be able to test a bit things. I think it would be good to agree on how to provide this non-flattening option to Web developers. Is the suggestion of comment 5 enough? Any other idea?

Scrolling with touch events does not work at the moment with iframes even with that patch. As I see, 'overflow:auto' content creates ScrollingTreeOverflowScrollingNodeIOS nodes in order to capture these events. Should we add such mechanism for iframes too?

[Simon Fraser (smfr)]

(In reply to Frédéric Wang (:fredw) from comment #12)
> Is the suggestion of comment 5 enough? Any other idea?

No, it's much too late to use the viewport tag as a key to not frame flatten.

> Scrolling with touch events does not work at the moment with iframes even
> with that patch. As I see, 'overflow:auto' content creates
> ScrollingTreeOverflowScrollingNodeIOS nodes in order to capture these
> events. Should we add such mechanism for iframes too?

There's a bunch of work that has to happen to make frames/iframes scrollable, involving educating the scrolling state tree & scrolling tree about scrollable frames (we don't even do that on Mac yet), making latching work correctly etc. Then the UI process has to be fixed to make UIScrollViews for scrollable frames.

I think the best course of action would be to make iframes fast-scrollable on Mac first, to set up that infrastructure.

[Frédéric Wang (:fredw)]

(In reply to Simon Fraser (smfr) from comment #13)
> There's a bunch of work that has to happen to make frames/iframes
> scrollable, involving educating the scrolling state tree & scrolling tree
> about scrollable frames (we don't even do that on Mac yet), making latching
> work correctly etc. Then the UI process has to be fixed to make
> UIScrollViews for scrollable frames.

Yes, that more or less matches my understanding after a first investigation of the code. Would that mean creating a new ScrollingNodeType for iframe/frame? Or re-using an existing one?

> I think the best course of action would be to make iframes fast-scrollable
> on Mac first, to set up that infrastructure.

I see. Please cc' me if there are Bugzilla entries regarding this or similar TODO.

[Rick Byers]

> Turning off frame flattening on iOS is a big usability change and needs to be done with caution.

Simon, I know we chatted briefly in Tokyo about this, but I'm curious to better understand your thinking here.  Why is this a greater usability risk than supporting overflow:scroll divs?

As I mentioned, I don't think I've ever heard a single user complain in Chrome about getting stuck in an iframe scroll chain.  We do occasionally hear complaints of users getting stuck in regions that cancel all the touch events (eg. it's possible to fling-pinch to zoom in on an area completely covered by a map, and now be unable to zoom/pan the viewport with no recourse other than reload).

Certainly nested scrollers are generally a bad UX paradigm, but virtually all mobile-optimized sites already avoid them, as do many desktop sites using scrollable iframes (eg. think of the gmail compose window or JSBin.com).  App-like layouts are increasingly common where the main document isn't scrollable at all, but there are some single elements (div or iframe) which are.  If it would help I could add some metrics to Chrome to track how often users encounter such nested scroller scenarios on Android and (after giving some time for the metrics to reach a non-trivial fraction of our users) report numbers here.

[Frédéric Wang (:fredw)]

(In reply to Simon Fraser (smfr) from comment #13)
> There's a bunch of work that has to happen to make frames/iframes
> scrollable, involving educating the scrolling state tree & scrolling tree
> about scrollable frames (we don't even do that on Mac yet), making latching
> work correctly etc. Then the UI process has to be fixed to make
> UIScrollViews for scrollable frames.
> 
> I think the best course of action would be to make iframes fast-scrollable
> on Mac first, to set up that infrastructure.

@Simon: I did more analysis on this (see https://people.igalia.com/fwang/ios/class-hierarchy/ for my notes) and I have some questions/comments:

0) Can you elaborate on what you mean by "iframe fast-scrollable on Mac"? I understand it means creating scrolling node for the iframe in order to handle user interaction in a separate process/thread. Is that correct?

1) There are RenderFrame/RenderIFrame::requiresLayer and RenderLayerCompositor::requiresCompositingForFrame functions involved in the decision of whether the frame will have a RenderLayer or RenderLayerBacking attached to the iframe/frame renderer. I believe we should change these functions somehow to force that attachment?

2) Similarly, it seems that the condition RenderLayer::hasTouchScrollableOverflow (iOS) or RenderLayer::needsCompositedScrolling (Mac) to create a scrolling node should be changed to take into consideration the frame/iframe.

3) Repeating comment 14, should we add another ScrollingNodeType for iframe/frame? Or re-using an existing one?

4) One hesitation I have is whether we want the scrolling node for the iframe/frame's renderer itself, or for RenderView attached to the document loaded into that iframe/frame?

Thanks

[Simon Fraser (smfr)]

(In reply to Frédéric Wang (:fredw) from comment #16)
> (In reply to Simon Fraser (smfr) from comment #13)
> > There's a bunch of work that has to happen to make frames/iframes
> > scrollable, involving educating the scrolling state tree & scrolling tree
> > about scrollable frames (we don't even do that on Mac yet), making latching
> > work correctly etc. Then the UI process has to be fixed to make
> > UIScrollViews for scrollable frames.
> > 
> > I think the best course of action would be to make iframes fast-scrollable
> > on Mac first, to set up that infrastructure.
> 
> @Simon: I did more analysis on this (see
> https://people.igalia.com/fwang/ios/class-hierarchy/ for my notes) and I
> have some questions/comments:
> 
> 0) Can you elaborate on what you mean by "iframe fast-scrollable on Mac"? I
> understand it means creating scrolling node for the iframe in order to
> handle user interaction in a separate process/thread. Is that correct?

This involves:
1. Creating the right layer hierarchy with tiled backing store for the iframe contents, as we do for the main frame.
2. Creating ScrollingStateFrameScrollingNodes for them
3. Making sure the scrolling tree handles nested ScrollingStateFrameScrollingNodes properly
4. Not putting iframe's ScrollableAreas into the non-fast scrollable region
4. Teaching the scrolling thread to direct a mouse wheel at the appropriate ScrollingTreeFrameScrollingNode
5. Making sure that scroll latching works
6. Probably more

> 1) There are RenderFrame/RenderIFrame::requiresLayer and
> RenderLayerCompositor::requiresCompositingForFrame functions involved in the
> decision of whether the frame will have a RenderLayer or RenderLayerBacking
> attached to the iframe/frame renderer. I believe we should change these
> functions somehow to force that attachment?

Yes, those fast-scrollable frames/iframes will need to be composited in their parent document.

> 2) Similarly, it seems that the condition
> RenderLayer::hasTouchScrollableOverflow (iOS) or
> RenderLayer::needsCompositedScrolling (Mac) to create a scrolling node
> should be changed to take into consideration the frame/iframe.

Those are more for overflow:scroll. I think scrollable frames/iframes would not use this code.

> 3) Repeating comment 14, should we add another ScrollingNodeType for
> iframe/frame? Or re-using an existing one?

Scrolling{State|Tree}FrameScrollingNode should suffice.

> 4) One hesitation I have is whether we want the scrolling node for the
> iframe/frame's renderer itself, or for RenderView attached to the document
> loaded into that iframe/frame?

The scrolling node should get hooked up with the layers inside the frame/iframe, just like for the main page.

[Frédéric Wang (:fredw)]

(In reply to Simon Fraser (smfr) from comment #13)
> I think the best course of action would be to make iframes fast-scrollable
> on Mac first, to set up that infrastructure.

I'm moving this preliminary step to bug 171667.

[Frédéric Wang (:fredw)]

(In reply to Simon Fraser (smfr) from comment #13)
> (In reply to Frédéric Wang (:fredw) from comment #12)
> > Is the suggestion of comment 5 enough? Any other idea?
> 
> No, it's much too late to use the viewport tag as a key to not frame flatten.
> 

@Simon: What about checking the "-webkit-overflow-scrolling: touch;" property on the iframe/frame elements?

[Simon Fraser (smfr)]

(In reply to Frédéric Wang (:fredw) from comment #19)
> @Simon: What about checking the "-webkit-overflow-scrolling: touch;"
> property on the iframe/frame elements?

I really don't want to propagate use of that property.

[Frédéric Wang (:fredw)]

Created attachment 309487 [details]
Do not flatten iframes with viewport units.

[Frédéric Wang (:fredw)]

Created attachment 309488 [details]
Do not flatten iframes with viewport units.

[Frédéric Wang (:fredw)]

Created attachment 312982 [details]
Small patch to disable frame flattening on iOS

[Frédéric Wang (:fredw)]

Created attachment 313838 [details]
Test Patch to disable frame flattening and enable async frame scrolling

[Frédéric Wang (:fredw)]

So just to follow-up here, I have a first experimental set of patches to make iframe scrollables:

attachment 314570 [details]
attachment 314559 [details]
attachment 314574 [details]
attachment 314603 [details]
attachment 173644 [details]
attachment 313838 [details]

[Frédéric Wang (:fredw)]

Created attachment 316536 [details]
Test Patch to disable frame flattening and enable async frame scrolling

Rebasing.

[Build Bot]

Comment on attachment 316536 [details]
Test Patch to disable frame flattening and enable async frame scrolling

Attachment 316536 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/4228129

New failing tests:
fast/images/low-memory-decode.html

[Build Bot]

Created attachment 316760 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Jacek Bogdański]

Hello,

It would be nice to get some information at what stage is this issue. Currently, on iOS 11.3.1 we have to wrap iframe element with div container to make it scrollable: 

<style>
div {
  overflow-y: auto;
  -webkit-overflow-scrolling: touch;
}
</style>

<div><iframe src="..."></iframe></div>

As I understand this ticket should also cover above issue (not only scrolling=yes attribute).

[Frédéric Wang (:fredw)]

(In reply to Jacek Bogdański from comment #29)
> As I understand this ticket should also cover above issue (not only
> scrolling=yes attribute).

Hello Jacek, if I understand your question correctly you are saying that even with the default value (scrolling=auto), iframes should be scrollable. If so the answer is yes and that's already handled in the current set of patches. I'm updating the title to clarify it.

[Jacek Bogdański]

That's right. Thank you for your clarification.

[Antti Koivisto]

Scrollable frames on iOS were enabled in https://trac.webkit.org/changeset/242814

[Maxim Tsoy]

Does this mean we will be able to have unexpanded scrollable iframes in the next iOS?

[Frédéric Wang (:fredw)]

(In reply to Maxim Tsoy from comment #33)
> Does this mean we will be able to have unexpanded scrollable iframes in the
> next iOS?

Apple does not comment on future releases...
You can check the beta: https://beta.apple.com/sp/betaprogram

[Maxim Tsoy]

(In reply to Frédéric Wang (:fredw) from comment #34)
> Apple does not comment on future releases...
> You can check the beta: https://beta.apple.com/sp/betaprogram

Makes sense, thanks. But technically it is ready, and we just need to wait for Apple to include this change in the release, right? Or is there still some work to be done? I see there are items in "Depends on".

[Antti Koivisto]

Yes, scrollable frames are enabled on iOS/iPadOS 13 in Safari and modern API clients (WKWebView or SFSafariViewController).

[Maxim Tsoy]

Just checked on iOS beta, it seems to be working. After 3 years of maintaining an ugly hack with a scrollable container and proxied scroll events, our struggle is over.
Surfly dev team is jumping in happiness and we'll probably throw a party in the name of this fix. Thank you guys!

[Frédéric Wang (:fredw)]

(In reply to Maxim Tsoy from comment #35)
> (In reply to Frédéric Wang (:fredw) from comment #34)
> Or is there still
> some work to be done? I see there are items in "Depends on".

I think most of the work is done. I'm removing the "depends on" that no longer applies.