RESOLVED CONFIGURATION CHANGED 149264
[iOS] IFrames are not scrollable even when scrolling=yes is specified
https://bugs.webkit.org/show_bug.cgi?id=149264
Summary [iOS] IFrames are not scrollable even when scrolling=yes is specified
Dima Voytenko
Reported 2015-09-17 10:10:48 PDT
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.
Attachments
testcase (313 bytes, text/html)
2017-04-20 03:22 PDT, Frédéric Wang (:fredw)
no flags
Do not flatten iframes (3.43 KB, patch)
2017-04-28 00:27 PDT, Frédéric Wang (:fredw)
no flags
Do not flatten iframes with viewport units. (975 bytes, patch)
2017-05-09 02:52 PDT, Frédéric Wang (:fredw)
no flags
Do not flatten iframes with viewport units. (977 bytes, patch)
2017-05-09 02:55 PDT, Frédéric Wang (:fredw)
no flags
Small patch to disable frame flattening on iOS (1019 bytes, patch)
2017-06-15 08:12 PDT, Frédéric Wang (:fredw)
no flags
Test Patch to disable frame flattening and enable async frame scrolling (1.37 KB, patch)
2017-06-26 08:07 PDT, Frédéric Wang (:fredw)
no flags
Test Patch to disable frame flattening and enable async frame scrolling (1.37 KB, patch)
2017-07-27 02:02 PDT, Frédéric Wang (:fredw)
buildbot: commit-queue-
Archive of layout-test-results from ews124 for ios-simulator-wk2 (967.81 KB, application/zip)
2017-07-31 02:52 PDT, Build Bot
no flags
Rick Byers
Comment 1 2015-09-18 08:28:43 PDT
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)
Comment 2 2015-09-18 08:35:59 PDT
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
Comment 3 2015-09-18 08:38:08 PDT
(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
Comment 4 2015-09-18 09:13:08 PDT
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)
Comment 5 2015-09-28 17:44:23 PDT
Ojan suggested not flattening frames on pages with viewport tags.
David Kilzer (:ddkilzer)
Comment 6 2015-10-02 20:39:32 PDT
Ries
Comment 7 2016-12-22 14:21:32 PST
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)
Comment 8 2017-04-20 03:22:52 PDT
Created attachment 307586 [details] testcase
Frédéric Wang (:fredw)
Comment 9 2017-04-28 00:27:09 PDT
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)
Comment 10 2017-04-28 09:05:44 PDT
Turning off frame flattening on iOS is a big usability change and needs to be done with caution.
zalan
Comment 11 2017-04-28 09:07:28 PDT
(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)
Comment 12 2017-04-28 09:26:01 PDT
(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)
Comment 13 2017-04-28 10:56:18 PDT
(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)
Comment 14 2017-04-28 11:22:14 PDT
(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
Comment 15 2017-04-28 19:17:17 PDT
> 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)
Comment 16 2017-05-03 09:35:03 PDT
(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)
Comment 17 2017-05-03 13:06:59 PDT
(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)
Comment 18 2017-05-04 09:58:47 PDT
(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)
Comment 19 2017-05-05 11:34:16 PDT
(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)
Comment 20 2017-05-05 13:06:04 PDT
(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)
Comment 21 2017-05-09 02:52:45 PDT
Created attachment 309487 [details] Do not flatten iframes with viewport units.
Frédéric Wang (:fredw)
Comment 22 2017-05-09 02:55:04 PDT
Created attachment 309488 [details] Do not flatten iframes with viewport units.
Frédéric Wang (:fredw)
Comment 23 2017-06-15 08:12:23 PDT
Created attachment 312982 [details] Small patch to disable frame flattening on iOS
Frédéric Wang (:fredw)
Comment 24 2017-06-26 08:07:27 PDT
Created attachment 313838 [details] Test Patch to disable frame flattening and enable async frame scrolling
Frédéric Wang (:fredw)
Comment 25 2017-07-05 07:53:03 PDT
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)
Comment 26 2017-07-27 02:02:07 PDT
Created attachment 316536 [details] Test Patch to disable frame flattening and enable async frame scrolling Rebasing.
Build Bot
Comment 27 2017-07-31 02:52:01 PDT
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
Comment 28 2017-07-31 02:52:02 PDT
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
Comment 29 2018-05-09 04:54:55 PDT
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)
Comment 30 2018-05-09 05:05:16 PDT
(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
Comment 31 2018-05-09 05:07:24 PDT
That's right. Thank you for your clarification.
Antti Koivisto
Comment 32 2019-07-09 01:34:40 PDT
Scrollable frames on iOS were enabled in https://trac.webkit.org/changeset/242814
Maxim Tsoy
Comment 33 2019-07-09 02:23:58 PDT
Does this mean we will be able to have unexpanded scrollable iframes in the next iOS?
Frédéric Wang (:fredw)
Comment 34 2019-07-09 02:44:26 PDT
(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
Comment 35 2019-07-09 04:08:06 PDT
(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
Comment 36 2019-07-09 05:00:38 PDT
Yes, scrollable frames are enabled on iOS/iPadOS 13 in Safari and modern API clients (WKWebView or SFSafariViewController).
Maxim Tsoy
Comment 37 2019-07-09 09:38:52 PDT
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)
Comment 38 2019-07-10 03:54:49 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.