Amphtml: Images iterate unexpectedly rapidly with amp-carousel autoslide - IOS Safari 10.3 BETA

[kuma]

Short description of your issue:

Images iterate unexpectedly rapidly when I use amp-carousel autoslide

How do we reproduce the issue?

1. Open a website that is using amp-carousel with autoslide
   https://www.google.co.jp/amp/s/www.hotpepper.jp/strJ000003128/amp/?client=safari

What browsers are affected?

Safari on iOS 10.3 beta 2

Which AMP version is affected?

Version 1487358851767

I've uploaded a video capture of the issue below.
https://youtu.be/_kxfs8duGMw

[mishelp]

We are experiencing the same issue with unexpectedly rapid and messy display of images in amp-carousel auto-slide

Short description of your issue:

On AMP page, the component "amp-carousel" starts showing images extremely fast after first change of the slide.
Screenshot:
https://www.screencast.com/t/koAROtcY

How do we reproduce the issue?

Open a website that is using amp-carousel with autoslide:
https://www.ttu.co.il/MP3-%D7%A7%D7%98%D7%9F-%D7%91%D7%9E%D7%99%D7%95%D7%97%D7%93-%D7%91%D7%A6%D7%91%D7%A2-%D7%99%D7%A8%D7%95%D7%A7.html?amp=1
Other page example with carousel:
https://www.ttu.co.il/Broadlink-RM-Mini-3-%D7%94%D7%A9%D7%9C%D7%98-%D7%94%D7%90%D7%95%D7%A0%D7%99%D7%91%D7%A8%D7%A1%D7%9C%D7%99-%D7%94%D7%97%D7%9B%D7%9D.html/?amp=1

What browsers are affected?

Google Chrome 56.0.2924.87 (64-bit)

Notes:

= HTML structure of the amp-carousel component is in accordance with ampproject requirements
= AMP page in question is a valid AMP page.

Please advise as we only have the amp-carousel component to blame.

Thanks,

[aghassemi]

I can reproduce 100% of the time on https://www.ttu.co.il/MP3-%D7%A7%D7%98%D7%9F-%D7%91%D7%9E%D7%99%D7%95%D7%97%D7%93-%D7%91%D7%A6%D7%91%D7%A2-%D7%99%D7%A8%D7%95%D7%A7.html?amp=1

@mishelp @kuma-san Thanks for reporting. Was this working for you previously and is a regression you noticed recently?

@ericlindley-g @camelburrito I am setting this to P1

[kuma]

@aghassemi
Thanks for accepting this issue!
For my URL, this is n regression I've noticed recently.
I could only reproduce 100% from iOS10.3 beta2 safari with the URL I've wrote in the issue.

[kuma]

I've updated my iPhone to iOS10.3 beta3, and it still reproduces the issue.

[camelburrito]

i figured out this issue -

its this

body{direction:rtl;unicode-bidi:embed;}

right now carousel does not work well with RTls

You could over come it by adding a dir=ltr on your carousel

Here is the JS Bin where it is working with the ltr fix.
http://jsbin.com/gasawaz

Dupe of: https://github.com/ampproject/amphtml/issues/5950

@ericlindley-g lets prioritize this during the upcoming sprints.

[kuma]

My website which I wrote URL above does not use RTL.
I believe that the cause of the issue is not the same as the @mishelp's one.

[camelburrito]

@kuma-san - I tested it on latest safari works fine, will have to get a beta device and test it and see if it works.

[kuma]

Please let me know if I can help to determine the core issue in any way.
I looked at the console but nothing was there...

[camelburrito]

I guess we have what we need ! we will have some one look into it.

[kuma]

Thanks @camelburrito

[camelburrito]

verified that this bug is real in IOS safari beta , i am guessing they messed up with snap points again.

@dvoytenko - snappoints are getting hard to manage, should we just always use the JS based scroll snapping?

@alanorozco do you have cycles to look at this ?

[dvoytenko]

@camelburrito So, this is only in beta? Can we see or send a Safari bug for this? I don't mind switching to JS snap-points only - that could be a fine tradeoff, but let's start with a webkit bug.

[camelburrito]

@dvoytenko - yes this is in beta only. Ill need to work on a JS bin replicating the bug. Let me see if i find some time to do that.

[mrjoro]

This bug is marked as P0, which implies people work on it basically nonstop until it gets fixed. Is this actually a P0?

[camelburrito]

yes

[mrjoro]

It has been 2 days since this was marked as a P0. Can you provide an update on the status?

[muxin]

I think it's a p1 because the beta is not public yet.

So, there are two approach:

1. figure out what's wrong with the native snap point and fix it.
2. get rid of snap point and use js in safari

I tried removing the snap point related css and the auto play works but once there's touch event, the scroll will stuck in the middle without aligning the slide. Still debugging this. I will go with whichever approach is easier first.

[dvoytenko]

If it's beta, then the question when beta goes into PROD. If in two weeks, I'd say, it's a P0.

[amphtml-team]

P0
On Wed, Mar 1, 2017 at 5:30 PM Dima Voytenko [e-mail]
wrote:

If it's beta, then the question when beta goes into PROD. If in two weeks,
I'd say, it's a P0.

—

You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/ampproject/amphtml/issues/7670#issuecomment-283527727,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANd2kGDGP0PQ-gLUcGOtrU-8qIcYZlnTks5rhhu7gaJpZM4MF0ds
.

--
You received this message because you are subscribed to the Google Groups
"amphtml-eng-github" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/a/google.com/d/msgid/amphtml-eng-github/ampproject/amphtml/issues/7670/283527727%40github.com
https://groups.google.com/a/google.com/d/msgid/amphtml-eng-github/ampproject/amphtml/issues/7670/283527727%40github.com?utm_medium=email&utm_source=footer
.

>

-- Sriram

[kuma]

fyi, still persists iOS 10.3 beta4

[camelburrito]

@kuma-san - yes we are working on it. Do you know when this is coming out of beta?

[kuma]

@camelburrito I'm sorry but I have no information on that.
I tested w/ iOS 10.3. beta 6 and still persists.

[tori3]

I tried debug. but I do not know the cause...

In iOS 10.3 beta, the callback of vsync.mutate is not called and the flag of snappingInProgress_ is not changed, and seems that updateOnScroll_ is repeatedly executed in 35 msec.

https://github.com/ampproject/amphtml/blob/master/extensions/amp-carousel/0.1/slidescroll.js#L458

Maybe in vsync it seems that the problem is related to bug of iOS 10.3 safari.

[cramforce]

Please paste the respective webkit bug number here.

[cramforce]

I just filed one https://bugs.webkit.org/show_bug.cgi?id=169800

[camelburrito]

The bug has not been fixed yet - we have just patched so that the carousel is at least usable. There are some issues with overscroll and slide skipping.

[camelburrito]

Lowered priority because we have a patch that sort of makes this usable.

[camelburrito]

@muxin - look at the comment here

https://bugs.webkit.org/show_bug.cgi?id=169800#c3

We will need to change the snap-points css to match the latest spec

[camelburrito]

We have fixed this for ios versions 10.3+ with some minor ux comromises and guarded with an experiment - slidescroll-disable-css-snap. If the webkit bug we reported is fixed then we will not have to launch the experiment. Else we will launch the experiment on or before the 10.3 launch date. (we are not rushing to patch the code to prod since the issue is in a beta version of IOS and we are not sure of a launch date). The fix will be in prod in 2 weeks (and we can flip the experiment after that).

[crocubot]

10.3 went into Prod yesterday

[kuma]

https://developer.apple.com/news/?id=03272017b
10.3 has released on Mar 27, and it works!
Just for the double check, I've also checked with Safari 10.3.2 beta 1, and it still works.

[crocubot]

@kuma-san No, this is still broken in 10.3. Check this example page on a 10.3 phone. The "Loopy!" carousel goes nuts.
http://amphtml-nightly.herokuapp.com/test/manual/amp-slidescroll.amp.html

[ericlindley-g]

@yowz — I can't repro the issue on iPhone 6 on Safari. What device are you using?

[crocubot]

@ericlindley-g I am on an iPhone 6 Plus in Safari. In that link above, if you hit the "Toggle CSS SNAP" button, does the problem occur? I'm wondering whether the slidescroll-disable-css-snap experiment has been turned on by default as a stopgap fix.

[cramforce]

Per @muxin slidescroll-disable-css-snap was activated in production yesterday at 7 PM PST.

[cramforce]

For dev channel users it was only turned on a few hours ago.

[crocubot]

Ah, good news. I must have already bucketed myself on the wrong side of the experiment. Thanks @cramforce and @muxin

[aghassemi]

Closing this and moving he remaining work to https://github.com/ampproject/amphtml/issues/8553