Bug 149264

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!

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.

(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!

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.

Ojan suggested not flattening frames on pages with viewport tags.

<rdar://problem/22961085>

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.

Created attachment 307586 [details] testcase

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

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

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

(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?

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

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

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

(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

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

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

(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?

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

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

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

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

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

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]

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

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

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

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

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

That's right. Thank you for your clarification.

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

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

(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

(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".

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

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!

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