|Summary:||Position fixed is buggy with overflow:auto scrolling inside iframes|
|Product:||WebKit||Reporter:||Dima Voytenko <dvoytenko>|
|Component:||New Bugs||Assignee:||Simon Fraser (smfr) <simon.fraser>|
|Severity:||Major||CC:||dvoytenko, fred.wang, jfernandez, j.straka, simon.fraser, webkit-bug-importer, webkit|
|Hardware:||iPhone / iPad|
|Bug Depends on:||176243, 175135, 179946|
Description Dima Voytenko 2016-02-18 10:35:22 PST
To reproduce: 1. Open http://output.jsbin.com/nayoxe/quiet on iOS 2. Try to scroll fast Observe: Top and bottom elements styled with `position:fixed` jump and flicker. Expected: `position:fixed` elements should stay put. Description: This is a somewhat specific example. It's uses `overflow:auto` scrolling inside of an iframed document, which is a workaround for the fact that iOS doesn't allow iframes to scroll. Source code: http://jsbin.com/nayoxe/edit?html,css,output Please advise.
Comment 1 Radar WebKit Bug Importer 2016-02-19 09:12:34 PST
Comment 2 Frédéric Wang (:fredw) 2017-05-24 04:23:41 PDT
Important note: it seems that this bug is much more obvious with real device than with the simulator. I was wondering whether bug 172349 could be involved here, but I don't really see much difference on the simulator (again it would be good to try it on real device to be really sure).
Comment 3 Simon Fraser (smfr) 2017-05-24 15:33:21 PDT
I don't see severe jumping or flicking of the top and bottom bars in that testcase, on 10.3.2. I do see some subpixel jittering, which is because WebCore rounds scroll positions to integral values, and when the page has a non-unit scale there's some rounding going on. On what device and iOS version do you see the more severe flickering? I don't think this is related to bug 172349.
Comment 4 Frédéric Wang (:fredw) 2017-05-30 22:42:59 PDT
(In reply to Simon Fraser (smfr) from comment #3) > I don't see severe jumping or flicking of the top and bottom bars in that > testcase, on 10.3.2. I do see some subpixel jittering, which is because > WebCore rounds scroll positions to integral values, and when the page has a > non-unit scale there's some rounding going on. > > On what device and iOS version do you see the more severe flickering? OK, I was actually testing with the simulator version 10.3.2 (where I see the behavior you describe) and the testing on a real device was done by dvoytenko and jfernandez (where they see the bug). So maybe it's actually related to the iOS version?
Comment 5 Simon Fraser (smfr) 2017-05-30 23:04:43 PDT
This code changed when I implemented visual viewports in 10.3, so it's quite possible it changed. Please re-open if you can reproduce on 10.3 or later.
Comment 6 Javier Fernandez 2017-06-01 00:28:41 PDT
I've been trying to reproduce the issue in iOS 10.3.2 and, even that there are some artifacts, the big jumps seem to have gone. I can't be sure about some subtle flickering, but that could be a different issue.
Comment 7 Dima Voytenko 2017-06-02 12:50:02 PDT
Created attachment 311845 [details] Visual effects with position:fixed
Comment 8 Dima Voytenko 2017-06-02 12:50:26 PDT
Created attachment 311846 [details] More visual effects with position:fixed
Comment 9 Dima Voytenko 2017-06-02 12:51:27 PDT
I'll reopen. There are still a fair amount of visual effects. I've put together an example in http://output.jsbin.com/hatega/quiet. The screen recordings are attached.
Comment 10 Simon Fraser (smfr) 2017-06-02 13:33:51 PDT
Thanks for the data.
Comment 11 Frédéric Wang (:fredw) 2017-07-21 00:30:53 PDT
I just tested Dima's test cases: http://output.jsbin.com/hatega/quiet https://output.jsbin.com/tegomaj/quiet with a development build of Safari ( https://trac.webkit.org/changeset/219679/webkit ) and I see the flickering and big jumps with the simulator. See https://people.igalia.com/fwang/ios/2017-07-21/ @Simon: Are you able to reproduce the issue too?
Comment 12 Simon Fraser (smfr) 2017-07-26 12:51:26 PDT
Yes, I can.
Comment 13 Simon Fraser (smfr) 2017-07-26 13:25:36 PDT
Created attachment 316469 [details] Testcase that works This doesn't happen with all position:fixed inside overflow scroll. This attachment works fine.
Comment 14 Simon Fraser (smfr) 2017-07-26 13:33:47 PDT
So both jsbin examples suffer from being inside an <iframe>. Do you have content that shows this bug outside of an <iframe>?
Comment 15 Frédéric Wang (:fredw) 2017-07-27 09:12:01 PDT
Created attachment 316551 [details] Reduced testcase (flickering) This is a reduced testcase for flickering. Indeed, if I don't embed the content inside an iframe this flickering bug goes away, so the iframe seems involved here. Note: For now I have not been able to find which content added by AMP is causing the big jumps.
Comment 16 Frédéric Wang (:fredw) 2017-07-28 03:33:49 PDT
(In reply to Frédéric Wang (:fredw) from comment #15) > Note: For now I have not been able to find which content added by AMP is > causing the big jumps. I tried reducing the test further this morning, removing all the unnecessary generated CSS and markup. This is the best I was able to get for now: https://people.igalia.com/fwang/ios/position-fixed-bugs/flicker-and-jump.html IIUC, AMP creates a second HTML element which is used as a workaround for bug 5991 without breaking CSS selectors. However, it is very difficult to work on the minified AMP file in order to remove or simplify this element or even just understand when its scroll* properties are read/set (which might confuse the scrolling). @Dima: Are you able to provide a testcase for the jumps that does not involve the minified AMP script (similar to attachment 316551 [details] for flickering)? Or at least where can I find a non-minified version of the AMP script?
Comment 17 Dima Voytenko 2017-07-28 17:02:50 PDT
From what I can tell, this is because of a touch event on the body. See reduced example on http://output.jsbin.com/fusijum/quiet All our touch events are always passive, but I don't think we can communicate this to WebKit yet.
Comment 18 Simon Fraser (smfr) 2017-07-28 17:44:43 PDT
WebKit does support passive event listener registration.
Comment 19 Dima Voytenko 2017-07-28 18:41:42 PDT
I'll retest with passive then. Not 100% sure that'd help yet.
Comment 20 Frédéric Wang (:fredw) 2017-07-29 01:19:29 PDT
(In reply to Dima Voytenko from comment #17) > From what I can tell, this is because of a touch event on the body. See > reduced example on http://output.jsbin.com/fusijum/quiet > > All our touch events are always passive, but I don't think we can > communicate this to WebKit yet. Thanks. I just tested it and I can reproduce the "big jumps" with a release build, but not with a nightly build. So maybe more things from the js file of https://people.igalia.com/fwang/ios/position-fixed-bugs/flicker-and-jump.html are involved here...
Comment 21 Simon Fraser (smfr) 2017-07-31 12:03:33 PDT
At least part of the problem here is that constraints passed to ScrollingStateFixedNode::updateConstraints() are computed using the fixed layout rect for the iframe (which is correct), but ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll() just grabs scrollingTree().fixedPositionRect() which is for the main frame.
Comment 22 Frédéric Wang (:fredw) 2017-08-02 07:28:37 PDT
(In reply to Simon Fraser (smfr) from comment #21) > At least part of the problem here is that constraints passed to > ScrollingStateFixedNode::updateConstraints() are computed using the fixed > layout rect for the iframe (which is correct), but > ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll() just > grabs scrollingTree().fixedPositionRect() which is for the main frame. So trying to understand what you mean: Basically, the fixed layout rect for the iframe is passed to the fixed node with the following trace: ScrollingStateFixedNode::updateConstraints AsyncScrollingCoordinator::updateNodeViewportConstraints RenderLayerCompositor::updateScrollCoordinatedLayer Then ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll recursively calls updateLayersAfterAncestorChange, passing the scrollingTree().fixedPositionRect() which for fixed nodes ends up at the following trace: FixedPositionViewportConstraints::layerPositionForViewportRect ScrollingTreeFixedNode::updateLayersAfterAncestorChange ... Naturally, the new fixed rect and position can be different from the ScrollingConstraints::m_*AtLastLayout values and are updated by updateLayersAfterAncestorChange. But as you said, the passed value scrollingTree().fixedPositionRect() refers to the main frame not the iframe. I'll upload a patch that tries to copy what is done in ScrollingTreeFrameScrollingNodeIOS::updateChildNodesAfterScroll.
Comment 23 Frédéric Wang (:fredw) 2017-08-02 07:31:05 PDT
Created attachment 316959 [details] Patch (fixedPositionRect ScrollingTreeOverflowScrollingNodeIOS::updateChildNodesAfterScroll) This patch tries to use the fixed rect of the first the ancestor iframe, copying the logic in ScrollingTreeFrameScrollingNodeIOS::updateChildNodesAfterScroll. This seems to get rid of the "flickering" (and "disappear") effect but does not solve the big jumps.