Bug 160622

Summary: [Scroll Snap] Scroll snap may not happen when scrollLeft is set programmatically
Product: WebKit Reporter: Sriram Krishnan <sriramkrish85>
Component: CSSAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Critical CC: dvoytenko, fred.wang, mrobinson, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Safari 9   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=283707
Attachments:
Description Flags
testcase
none
testcase
none
Another testcase none

Sriram Krishnan
Reported 2016-08-05 17:32:09 PDT
I am trying to hide the webkit scrollbar (i cannot use display: none or visibility: hidden due to - https://bugs.webkit.org/show_bug.cgi?id=160442) When i do something like div#outer::-webkit-scrollbar { height: 0px !important; } and then in JS do outer.scrollLeft = inner1.offsetWidth; The scrollLeft is being ignored (happens in safari and not on chrome) (now this starts working fine when i set height: 0.1px) Here is my JS BIN http://jsbin.com/fiwiqo HTML ======== <!DOCTYPE html> <html> <head> <meta charset="utf-8"> <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=0"/> <title>ScrollBar Safari bug</title> </head> <body> <div id="container"> <div id="outer" class="outer"> <div class="start-marker"></div> <div class="end-marker"></div> <div id="inner1" class="inner">1</div> <div id="inner2" class="inner">2</div> <div id="inner3" class="inner">3</div> </div> </div> <textarea id="log-el"></textarea> </body> </html> CSS ========= * { padding: 0; margin:0; } li { list-style-type: none; } body { background: orange; } /* AMP.CSS */ html, body { overflow-x: hidden !important; height: auto !important; } /** * Margin:0 is currently needed for iOS viewer embeds. * See: * https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md * and {@link ViewportBindingNaturalIosEmbed_} for more info. */ body { margin: 0 !important; } /* END AMP.CSS */ #container { width: 100vw; height: 300px; position: absolute; } #outer { background: red; box-sizing: border-box; display: flex !important; flex-wrap: nowrap; height: 100% !important; left: 0; overflow-x: auto !important; overflow-y: hidden !important; position: absolute !important; top: 0; white-space: nowrap !important; width: 100% !important; -webkit-overflow-scrolling: touch; -webkit-scroll-snap-type: mandatory; scroll-snap-type: mandatory; } .inner { background: yellow; border: solid 1px blue; font-size: 200px; display: flex !important; align-items: center !important; flex: 0 0 100% !important; height: 100% !important; justify-content: center !important; position: relative !important; -webkit-scroll-snap-coordinate: 0 0; scroll-snap-coordinate: 0 0; width: 100% !important; } .start-marker, .end-marker { background-color: transparent !important; display: block !important; flex: 0 0 1px !important; height: 100% !important; position: relative !important; -webkit-scroll-snap-coordinate: 0 0; scroll-snap-coordinate: 0 0; width: 1px !important; } .start-marker { order: -1 !important; margin-left: -1px !important; } end-marker { order: 100000000 !important; margin-right: -1px !important; } /* Let's get this party started */ div#outer::-webkit-scrollbar { height: 0px !important; } #log-el { background-color: cyan; display: block; width: 100%; height: 40vh; margin-top: 310px; } JS ======== var startTime = new Date().getTime(); function log(m) { var el = document.getElementById('log-el'); el.value = '[' + (new Date().getTime() - startTime) + '] ' + (m || '') + '\n'+ el.value; } outer.addEventListener('scroll', function(event) { log('scroll left: '+ outer.scrollLeft); }); outer.scrollLeft = inner1.offsetWidth;
Attachments
testcase (932 bytes, text/html)
2017-04-25 03:29 PDT, Frédéric Wang (:fredw) 		no flags
Details
testcase (1.19 KB, text/html)
2017-04-26 00:21 PDT, Frédéric Wang (:fredw) 		no flags
Details
Another testcase (1.01 KB, text/html)
2018-02-21 08:40 PST, Frédéric Wang (:fredw) 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2016-10-06 15:17:34 PDT
<rdar://problem/28660060>
Frédéric Wang (:fredw)
Comment 2 2017-04-25 03:29:44 PDT
Created attachment 308082 [details] testcase This is a reduced testcase based on the bug description. Scrolling works as expected in Safari and iOS. However, I still see the bug on https://output.jsbin.com/fiwiqo ; it would be good to have a reduced testcase demonstrating the problem and to update the bug description accordingly.
Sriram Krishnan
Comment 3 2017-04-25 10:46:55 PDT
:fredw - on IOS 10.3 the JS scroll works on MY JS BIN but it fails to snap to the right point after the scroll - should i open another bug for that?
Frédéric Wang (:fredw)
Comment 4 2017-04-25 12:43:06 PDT
(In reply to Sriram Krishnan from comment #3) > :fredw - on IOS 10.3 the JS scroll works on MY JS BIN but it fails to snap > to the right point after the scroll - should i open another bug for that? Yes, I'm able to reproduce the issue with your JS BIN, however the test case seems complex with many elements, css rules and javascript. Would it be possible to have a test with only the minimal code demonstrating the error (I tried attachment 308082 [details] based on the bug description but it does not work exhibit the issue)? At worst, I can try again to write one, starting from your JS BIN rather rather than from the bug description...
Sriram Krishnan
Comment 5 2017-04-25 13:37:40 PDT
(In reply to Frédéric Wang (:fredw) from comment #4) > (In reply to Sriram Krishnan from comment #3) > > :fredw - on IOS 10.3 the JS scroll works on MY JS BIN but it fails to snap > > to the right point after the scroll - should i open another bug for that? > > Yes, I'm able to reproduce the issue with your JS BIN, however the test case > seems complex with many elements, css rules and javascript. Would it be > possible to have a test with only the minimal code demonstrating the error > (I tried attachment 308082 [details] based on the bug description but it > does not work exhibit the issue)? At worst, I can try again to write one, > starting from your JS BIN rather rather than from the bug description... I think the issue happens in the presence if css snap points and your test case does not seem to have it
Wenson Hsieh
Comment 6 2017-04-25 13:50:25 PDT
(In reply to Sriram Krishnan from comment #5) > (In reply to Frédéric Wang (:fredw) from comment #4) > > (In reply to Sriram Krishnan from comment #3) > > > :fredw - on IOS 10.3 the JS scroll works on MY JS BIN but it fails to snap > > > to the right point after the scroll - should i open another bug for that? > > > > Yes, I'm able to reproduce the issue with your JS BIN, however the test case > > seems complex with many elements, css rules and javascript. Would it be > > possible to have a test with only the minimal code demonstrating the error > > (I tried attachment 308082 [details] based on the bug description but it > > does not work exhibit the issue)? At worst, I can try again to write one, > > starting from your JS BIN rather rather than from the bug description... > > I think the issue happens in the presence if css snap points and your test > case does not seem to have it Sriram, what is the expected vs. observed behavior in your test page? I loaded your test page on ToT WebKit on iOS. If I replace the following: `-webkit-scroll-snap-coordinate: 0 0` => `scroll-snap-align: start` `-webkit-scroll-snap-type: mandatory` => `scroll-snap-type: x mandatory` ...then scroll snapping behaves normally, and the test page loads with the box labeled "2" scrolled into view. Is this correct?
Sriram Krishnan
Comment 7 2017-04-25 14:24:54 PDT
(In reply to Wenson Hsieh from comment #6) > (In reply to Sriram Krishnan from comment #5) > > (In reply to Frédéric Wang (:fredw) from comment #4) > > > (In reply to Sriram Krishnan from comment #3) > > > > :fredw - on IOS 10.3 the JS scroll works on MY JS BIN but it fails to snap > > > > to the right point after the scroll - should i open another bug for that? > > > > > > Yes, I'm able to reproduce the issue with your JS BIN, however the test case > > > seems complex with many elements, css rules and javascript. Would it be > > > possible to have a test with only the minimal code demonstrating the error > > > (I tried attachment 308082 [details] based on the bug description but it > > > does not work exhibit the issue)? At worst, I can try again to write one, > > > starting from your JS BIN rather rather than from the bug description... > > > > I think the issue happens in the presence if css snap points and your test > > case does not seem to have it > > Sriram, what is the expected vs. observed behavior in your test page? > > I loaded your test page on ToT WebKit on iOS. If I replace the following: > > `-webkit-scroll-snap-coordinate: 0 0` => `scroll-snap-align: start` > `-webkit-scroll-snap-type: mandatory` => `scroll-snap-type: x mandatory` > > ...then scroll snapping behaves normally, and the test page loads with the > box labeled "2" scrolled into view. Is this correct? At ant point the box labeled 1 should be the first in view. So that is a bug, also CSS snap points are not working in when i replace as said in your comments. I put this in a JS Bin https://output.jsbin.com/xuhupof/quiet To re-clarify what works and what is broken on my initial JS bin, http://jsbin.com/fiwiqo/quiet Try scrolling the yellow container- CSS snap points work fine. Use the --> Button below the page , the container scrolls (which was not working before) but it fails to snap to the right snap point now.
Wenson Hsieh
Comment 8 2017-04-25 14:51:13 PDT
> At ant point the box labeled 1 should be the first in view. So that is a > bug, also I'm not sure I understand the reasoning behind this. Are you suggesting `outer.scrollLeft = inner1.offsetWidth;` in your script should not scroll the container such that the box labeled 2 is in view? > CSS snap points are not working in when i replace as said in your > comments. > I put this in a JS Bin > https://output.jsbin.com/xuhupof/quiet > Yes, they are not working because 10.3 is still on the old version of the spec (the version that still supports the deprecated -webkit-scroll-snap-coordinates property). Unfortunately, there's no way for you to try out these new properties on ToT WebKit on iOS. > > To re-clarify what works and what is broken on my initial JS bin, > http://jsbin.com/fiwiqo/quiet > > Try scrolling the yellow container- CSS snap points work fine. > Use the --> Button below the page , the container scrolls (which was not > working before) but it fails to snap to the right snap point now. I see -- thanks for the clarification! Two questions: 1. When you wrote "fails to snap to the right snap point", do you mean that it snaps to the wrong point, or that it does not snap at all? 2. This bug seems extremely similar to <https://bugs.webkit.org/show_bug.cgi?id=170560>. Can you help me understand what the difference between these two bugs is?
Sriram Krishnan
Comment 9 2017-04-25 15:03:36 PDT
(In reply to Wenson Hsieh from comment #8) > > At ant point the box labeled 1 should be the first in view. So that is a > > bug, also > > I'm not sure I understand the reasoning behind this. Are you suggesting > `outer.scrollLeft = inner1.offsetWidth;` in your script should not scroll > the container such that the box labeled 2 is in view? > > > CSS snap points are not working in when i replace as said in your > > comments. > > I put this in a JS Bin > > https://output.jsbin.com/xuhupof/quiet > > > > Yes, they are not working because 10.3 is still on the old version of the > spec (the version that still supports the deprecated > -webkit-scroll-snap-coordinates property). Unfortunately, there's no way for > you to try out these new properties on ToT WebKit on iOS. > > > > > To re-clarify what works and what is broken on my initial JS bin, > > http://jsbin.com/fiwiqo/quiet > > > > Try scrolling the yellow container- CSS snap points work fine. > > Use the --> Button below the page , the container scrolls (which was not > > working before) but it fails to snap to the right snap point now. > > I see -- thanks for the clarification! Two questions: > 1. When you wrote "fails to snap to the right snap point", do you mean that > it snaps to the wrong point, or that it does not snap at all? > 2. This bug seems extremely similar to > <https://bugs.webkit.org/show_bug.cgi?id=170560>. Can you help me understand > what the difference between these two bugs is? 1. It fails to snap at all (you can try out here https://output.jsbin.com/xuhupof/quiet) 2. in https://bugs.webkit.org/show_bug.cgi?id=170560 snapping happens after scroll but offsets to a wrong snap-point , here snapping does not happen at all
Wenson Hsieh
Comment 10 2017-04-25 19:42:09 PDT
(In reply to Sriram Krishnan from comment #9) > (In reply to Wenson Hsieh from comment #8) > > > At ant point the box labeled 1 should be the first in view. So that is a > > > bug, also > > > > I'm not sure I understand the reasoning behind this. Are you suggesting > > `outer.scrollLeft = inner1.offsetWidth;` in your script should not scroll > > the container such that the box labeled 2 is in view? > > > > > CSS snap points are not working in when i replace as said in your > > > comments. > > > I put this in a JS Bin > > > https://output.jsbin.com/xuhupof/quiet > > > > > > > Yes, they are not working because 10.3 is still on the old version of the > > spec (the version that still supports the deprecated > > -webkit-scroll-snap-coordinates property). Unfortunately, there's no way for > > you to try out these new properties on ToT WebKit on iOS. > > > > > > > > To re-clarify what works and what is broken on my initial JS bin, > > > http://jsbin.com/fiwiqo/quiet > > > > > > Try scrolling the yellow container- CSS snap points work fine. > > > Use the --> Button below the page , the container scrolls (which was not > > > working before) but it fails to snap to the right snap point now. > > > > I see -- thanks for the clarification! Two questions: > > 1. When you wrote "fails to snap to the right snap point", do you mean that > > it snaps to the wrong point, or that it does not snap at all? > > 2. This bug seems extremely similar to > > <https://bugs.webkit.org/show_bug.cgi?id=170560>. Can you help me understand > > what the difference between these two bugs is? > > > 1. It fails to snap at all (you can try out here > https://output.jsbin.com/xuhupof/quiet) Is this a regression? If so, on what version of iOS was it working before? > > 2. in https://bugs.webkit.org/show_bug.cgi?id=170560 snapping happens after > scroll but offsets to a wrong snap-point , here snapping does not happen at > all Regarding https://output.jsbin.com/xuhupof: Since you are on iOS10.3, specifying scroll-snap-type: x mandatory; will not work, since we don't support the new scroll snapping properties there. You'll have to use -webkit-scroll- snap-type: mandatory. It looks like the JS bin badge at the top right fading in and out is causing a layout to happen, which prompts WebKit to immediately scroll to the active snap offset (i.e. the nearest offset).
Sriram Krishnan
Comment 11 2017-04-25 21:36:44 PDT
(In reply to Wenson Hsieh from comment #10) > (In reply to Sriram Krishnan from comment #9) > > (In reply to Wenson Hsieh from comment #8) > > > > At ant point the box labeled 1 should be the first in view. So that is a > > > > bug, also > > > > > > I'm not sure I understand the reasoning behind this. Are you suggesting > > > `outer.scrollLeft = inner1.offsetWidth;` in your script should not scroll > > > the container such that the box labeled 2 is in view? > > > > > > > CSS snap points are not working in when i replace as said in your > > > > comments. > > > > I put this in a JS Bin > > > > https://output.jsbin.com/xuhupof/quiet > > > > > > > > > > Yes, they are not working because 10.3 is still on the old version of the > > > spec (the version that still supports the deprecated > > > -webkit-scroll-snap-coordinates property). Unfortunately, there's no way for > > > you to try out these new properties on ToT WebKit on iOS. > > > > > > > > > > > To re-clarify what works and what is broken on my initial JS bin, > > > > http://jsbin.com/fiwiqo/quiet > > > > > > > > Try scrolling the yellow container- CSS snap points work fine. > > > > Use the --> Button below the page , the container scrolls (which was not > > > > working before) but it fails to snap to the right snap point now. > > > > > > I see -- thanks for the clarification! Two questions: > > > 1. When you wrote "fails to snap to the right snap point", do you mean that > > > it snaps to the wrong point, or that it does not snap at all? > > > 2. This bug seems extremely similar to > > > <https://bugs.webkit.org/show_bug.cgi?id=170560>. Can you help me understand > > > what the difference between these two bugs is? > > > > > > 1. It fails to snap at all (you can try out here > > https://output.jsbin.com/xuhupof/quiet) > > Is this a regression? If so, on what version of iOS was it working before? > That is the spec and it is expected to work like that :) not sure if this is a regression > > > > 2. in https://bugs.webkit.org/show_bug.cgi?id=170560 snapping happens after > > scroll but offsets to a wrong snap-point , here snapping does not happen at > > all > > Regarding https://output.jsbin.com/xuhupof: > > Since you are on iOS10.3, specifying scroll-snap-type: x mandatory; will not > work, since we > don't support the new scroll snapping properties there. You'll have to use > -webkit-scroll- > snap-type: mandatory. > > It looks like the JS bin badge at the top right fading in and out is causing > a layout to > happen, which prompts WebKit to immediately scroll to the active snap offset > (i.e. the > nearest offset). https://output.jsbin.com/xuhupof/quiet - should not have the jsbin badge , the old scroll properties are working perfectly fine Pasting my comments from above -- To re-clarify what works and what is broken on my initial JS bin, http://jsbin.com/fiwiqo/quiet Try scrolling the yellow container- CSS snap points work fine. Use the --> Button below the page , the container scrolls (which was not working before) but it fails to snap to the right snap point now.
Frédéric Wang (:fredw)
Comment 12 2017-04-26 00:00:07 PDT
(In reply to Sriram Krishnan from comment #5) > I think the issue happens in the presence if css snap points and your test > case does not seem to have it Ah, OK. I understood you were doing the snapping yourself by setting the scrollLeft. I'll upload a new testcase.
Frédéric Wang (:fredw)
Comment 13 2017-04-26 00:21:45 PDT
Created attachment 308228 [details] testcase Sriram: So this is a new test case with CSS snap points. The scrollTop is set to move nearer to the green box than to the red box and the expectation would be that it will automatically snap to the green box, is that correct? If the user scrolls the #div1 in iOS ToT, the snapping happens correctly. However it fails when the scrollTop is set programmatically. #div2 is similar with a CSS syntax that works in Gecko. I see the same behavior: snapping happens when the user scrolls the div but fails when done programmatically. I tried to use a fixed div as in bug 170560 to force a reflow, but that did not change anything.
Frédéric Wang (:fredw)
Comment 14 2017-05-01 08:13:22 PDT
> Try scrolling the yellow container- CSS snap points work fine. The behavior when the user scrolls the yellow container is clear to me: the CSS snapping happens immediately after the scrolling end (mouse released on Desktop or "momentum" slowed down to zero speed on iOS). > Use the --> Button below the page , the container scrolls (which was not working before) but it fails to snap to the right snap point now. It is not clear to me when the CSS snapping should happen after a programmatic change? On the one hand, if it's just after setting the value then clicking "-->" will move the container somewhere between [1] and [2] and immediately go snap to [1] (assuming the move done by one clock is small, as in your demo). This seems to be the https://bugs.webkit.org/show_bug.cgi?id=170560 and what prevented the scrolling in the past. On the other hand, if it's not set immediately then I don't see how you can do the snapping without forcing a relayout yourself. This is what the JSBIN badge is doing in your demo. > That is the spec and it is expected to work like that :) Are you able to indicate which part of the specification clarify my doubt regarding snapping after programmatic change? It's possible that the behavior is unclear and an issue should be reported to the spec authors. Are you testing this on other platforms / browsers? What is the behavior there?
Frédéric Wang (:fredw)
Comment 15 2018-02-21 08:40:05 PST
Created attachment 334383 [details] Another testcase This is a simple testcase without -webkit prefixes and using the new values described in the latest Working Draft (https://drafts.csswg.org/css-scroll-snap/). The snapping happens when doing user scrolling with the iOS simulator (both the latest release and trunk) but not when doing programmatic scrolling. Chrome Canary (with experimental web platform features enabled) behaves the same.
Frédéric Wang (:fredw)
Comment 16 2018-02-21 10:11:03 PST
@Sriram: Concretely using attachment 334383 [details], what would be the expected behavior for you if you click several times the "Down" button to perform programmatic scrolling? I understand that: * Snapping should happen. * It should not happen for the first point (otherwise you are stuck) * It should happen for the second point... but should that happen immediately (after the first move) or once we have passed half the distance between the first and second point? * Once we snapped to the second point and continue to use the "Down" button then things start again from that point (we don't snap to the second point but to the third one) and so on for other points. As I said, I tested in the latest versions of WebKit and Chrome Canary and snapping does not happen at all for programmatic scrolling. Which platform do you use to get the behavior you want? I also read https://drafts.csswg.org/css-scroll-snap/#snap-concepts and it does not seem to say anything against snapping for programmatic scrolling. It is also a bit flexible/unclear about the exact implementation. It has an explanation for escaping the initial snap position that we could use as long as we claim that the initial programmatic move is not a "fairly small distance" 'User agents must ensure that a user can “escape” a snap position, regardless of the scroll method. For example, if the snap type is mandatory and the next snap position is more than two screen-widths away, a naïve “always snap to nearest” selection algorithm might “trap” the user if their end position was only one screen-width away. Instead, a smarter algorithm that only returned to the starting snap position if the end-point was a fairly small distance from it, and otherwise ignored the starting snap position, would give better behavior. (This implies that a scroll with only an intended direction must always ignore the starting snap positions.)' Note that for programmatic scroll with an intended direction (scrollBy), scrolling may be animated and hence interpreted like a user scroll. However, this does not correspond to the initial description of this bug which uses something like scrollLeft += 100.
Frédéric Wang (:fredw)
Comment 17 2018-04-02 06:48:28 PDT
Updating the title per comment 11
Frédéric Wang (:fredw)
Comment 18 2018-04-11 23:05:23 PDT
According to some AMP developers, this bug looks fixed in iOS 11.3. Moreover Blink and iOS now behave the same for attachment 334383 [details]. Hence I'm closing this bug. I understand that there is a use case with scroll snap and programmatic scrolling, but the spec does not seem very clear regarding when the snapping should happen exactly (see comment 16). Incidentally, this is not only for programmatic scrolling since one could replace the "scrollBy(...)" or "scrollLeft/Top += ..." Javascript calls with arrow keys to scroll the content. Probably, a new bug should be opened when the spec is clarified and we have an obvious spec violation or browser incompatibility.
Note You need to log in before you can comment on or make changes to this bug.