New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amp Viewer Does Not Display Text and Overlaps Content #11829

Closed
sarahgriffis opened this Issue Oct 27, 2017 · 18 comments

Comments

@sarahgriffis
Copy link

sarahgriffis commented Oct 27, 2017

What's the issue?

The amp-viewer incorrectly overlaps content and does not display fonts. This is only present on certain iOS / browser versions (see table below).

How do we reproduce the issue?

Via search:

  1. Google not rays pizza brooklyn
  2. Click on the AMP result.

Directly:

  1. https://www.google.com/amp/s/www.notrayspizza.com/

Notice the buttons View Full Menu and Order Now on Slice do not have text, sometimes eventually populate, and overlap other content.

No Text
png 2

Overlapping Content
png 1

No Text
png

What browsers are affected?

iOS version Chrome Version Safari Version buggy/good
10.3.2 (14F89) 61.0.3163.73   good
10.3.2 (14F89)   10.0 good
10.3.3 (14G60)   10.0 good
10.3.3 (14G60) 61.0.3163.73 10.0 good
10.2.1 (14D27)   10.0 good
11.0.3 (15A432)   11.0 buggy
11.0.3 61.0.3163   buggy
11.0.3   11.0 buggy
10.2 (14C92)   10.0 buggy
11.0.3 61.0.3163.73   buggy

Which AMP version is affected?

Amp Version 1508794187431

@dvoytenko

This comment has been minimized.

Copy link
Collaborator

dvoytenko commented Nov 1, 2017

/to @ericfs @jridgewell ptal

@ericfs

This comment has been minimized.

Copy link

ericfs commented Nov 1, 2017

Thanks for the detailed browser analysis.

This only happens in the viewer?

It looks like the page is no longer shown in the viewer because it is not valid.
https://www-notrayspizza-com.cdn.ampproject.org/c/s/www.notrayspizza.com/
https://search.google.com/test/amp?utm_source=gws&utm_medium=onebox&utm_campaign=suit&id=bkGJAbvaL1Ydd1llPkco0A

@ericfs

This comment has been minimized.

Copy link

ericfs commented Nov 3, 2017

Seems like a Safari rendering bug of some sort, but not sure exactly what. Poking the styles in the doc temporarily fixes the issue. I don't see a viewer side fix here.

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Nov 3, 2017

Can you get the page to load? It's still invalid for me.

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Nov 3, 2017

Oooo, you hit a really cool compositing GPU bug. I'm not able to reduce it to an easy to see base case yet, but it's definitely due to our iOS scroll hacks and the border-radius style you have on the buttons.

I'm calling in @dvoytenko. Maybe you can figure out a solution?

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Nov 3, 2017

Actually, it's the display: flex on the <amp-carousel>! Mind blowing.

@dvoytenko

This comment has been minimized.

Copy link
Collaborator

dvoytenko commented Nov 3, 2017

Wow! @jridgewell do you still have a repro case? I want to run it by Igalia folk and see if we can file a WebKit bug.

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Nov 3, 2017

https://output.jsbin.com/veficav/quiet

It's a really fun bug. The first button ("Order Now on Slice") reliably renders first for me. If we scroll down to the two buttons below the menu, they're reliably broken.

Select the span of one of the two buttons, toggle the color style. Both buttons will fix. But now the top button is broken. Scroll up, and toggle the same color style on the same button (the bottom one), and it'll fix the top button!

If you disable the display: flex on .i-amphtml-slides-container, all buttons are permanently fixed.

@dvoytenko

This comment has been minimized.

Copy link
Collaborator

dvoytenko commented Nov 3, 2017

Ok. I'll pass it along. What could be a fix though? I believe display:flex is pretty important on carousels.

/cc @camelburrito

@camelburrito

This comment has been minimized.

Copy link
Collaborator

camelburrito commented Nov 3, 2017

yeah carousel would lose functionality (looping) without flex

@dvoytenko

This comment has been minimized.

Copy link
Collaborator

dvoytenko commented Nov 3, 2017

@jridgewell Does this fail in iOS11?

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Nov 3, 2017

Only in iOS 11.

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Nov 3, 2017

@camelburrito: We need to figure out a CSS fix quickly, there's nothing about these buttons that makes them abnormal to other elements. It's very likely others publishers are hitting this bug on common elements and not even knowing it.

@sarahgriffis

This comment has been minimized.

Copy link

sarahgriffis commented Nov 4, 2017

@jridgewell We purposefully broken our AMP pages to be invalid until we're able to solve this and issue. Our CEO was on iOS 10.2 (14C92) and getting this bug, so it does seem to be present in iOS 10 as well.

Let me know if we can help! Thanks for looking into this!

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Nov 6, 2017

This has been forwarded to Apple. https://bugs.webkit.org/show_bug.cgi?id=179315

It looks like this is a serious regression, affecting both iframed pages and non-iframed (normal) pages.

@rudygalfi

This comment has been minimized.

Copy link
Contributor

rudygalfi commented Nov 13, 2017

It looks like the webkit bug https://bugs.webkit.org/show_bug.cgi?id=179315 has had a patch submitted. I'm going to go ahead and close this out. I'm unsure of the Safari release schedule, but please re-open if the problem persists.

@rudygalfi rudygalfi closed this Nov 13, 2017

@fred-wang

This comment has been minimized.

Copy link

fred-wang commented Mar 30, 2018

iOS 11.3 has been released yesterday and this bug seems to be fixed.

@jridgewell

This comment has been minimized.

Copy link
Member

jridgewell commented Apr 9, 2018

👏, agreed it seems to be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment