Amphtml: amp-access analytics on:visible does not trigger on iOS/Safari

[gremz]

What's the issue?

When using a on:visible amp-analytics trigger such as:
"triggers": { "accessPaywall": { "on": "visible", "request": "pageview", "selector": ".trb_apw_ph" } }
with the amp-access iframe implementation, the event does not trigger on iOS devices and Mac Safari. Scrolling appears to trigger the event.

How do we reproduce the issue?

An example exists here.

1. Navigate to the above mentioned Url in a Safari browser.
2. Notice that the request to https://analytics.example.com/no-content is not triggered
3. Scroll in the browser
4. Notice that the https://analytics.example.com/no-content event is triggered

What browsers are affected?

Tested on Mac Safari v11.0.3, iOS 10.3 and iOS 12.0

Which AMP version is affected?

This has been an issue. Currently tested against 1810281911260

[lannka]

Likely the same cause of #18972

@torch2424 pls take a look if your fix also work here.

[torch2424]

@lannka I don't see any use of amp-bind. After we get #19034 pulled, I can definitely try to run this through our proxy and see if it fixes this 😄

[dvoytenko]

@torch2424 Do we have a confirmation on this?

[lannka]

I believe amp-access might also use hidden attribute to show/hide stuff.

[torch2424]

@dvoytenko @lannka

So after a discussion in #19034 it was decided that we should only be using the hidden attribute on the element to toggle visibility, as mutation observer tracking can be expensive.

amp-access is not using hidden to hide any child elements, thus our mutation observer does not pick up the changes.

So this bug is still valid, and the fix would to add a hidden attribute on amp-access or its children. Also, I noticed that we are adding an amp-access-hidden attribute to amp-access, can we simply replace this with hidden? Or also add the hidden attribute?

P.S I ran this through the proxy, and can confirm the bug that way. But watched how the elements were being transformed and noticed the reason.

[dvoytenko]

Hmm. In the short-term, at least, I don't think amp-access and amp-subscriptions would be using hidden attributes. While somewhat related, the use-cases are still sufficiently different. That being said, there are major system events that trigger hide/show in those cases and they are well known. What should amp-access/amp-subscriptions do now to get picked up here?

[torch2424]

@dvoytenko

So @jridgewell encouraged that I open #19054 for this exact reason. It's on my TODO list to pick up after I finish some other things I am working on. We can simply add the other attribute cases there in the attributeFilter, but that being said, we don't want to watch too many. 😄

[jridgewell]

In the short-term, at least, I don't think amp-access and amp-subscriptions would be using hidden attributes.

What are the use cases? We can add both hidden and amp-access-hidden if the point is to give a CSS selector to hook onto.

[lannka]

+1 want to know more into the use cases.
overall, if possible we should simplify the contract between AMP extensions and InOb polyfill

[lannka]

@dvoytenko

[torch2424]

Pinging @dvoytenko for a response. We're currently doing a fixit week and it would be awesome if we could get this in 😄

[dvoytenko]

All! Subscription cases are pretty special and it's hard to manage them using hidden or similar tags. On the other hand, however, the events where content is shown/hidden are very obvious. Can we establish an API or a custom event to support such cases? FWIW, I thought Resources.mutateElement() should do this correctly already. That's the point of it, no?

[torch2424]

@dvoytenko

The reasoning behind using the mutation observer only for the hidden tag, is so that we don't watch all mutations such as: style, class, height, etc... We only wanted to watch visible vs. not visible.

After looking at Resources.mutateElement(), yes this would work for elements we control in terms of layout and things (and have an API to hook into this), but for an amp-bind case where it is controlled by the user, we would still need some way to efficiently determine if an element has changed visibility.

Perhaps we can make an API where you can pass specific attributes to watch? That way, for the case of amp-subscriptions, you can be explicit with which attributes will be causing visibility changes. But for our case in the intersection observer, we will default to the hidden attribute.

How does that sound? If that still won't work for subscriptions, perhaps we need different APIs for each?

[dvoytenko]

I think Resources.mutateELement() is that API that can (and should) handle all "special" cases that are not hidden attribute. Use cases such as amp-subscriptions have semantic markup, which do not directly state shown or hidden. E.g.

<section subscriptions-section="content">
  ...
</section>

This states that this section is "premium content" and is only visible when the user has authorization. It's hidden by default with CSS. Ideally we'd not require everyone to immediately set hidden attribute in the markup. Currently we do not require this and there are quite a few pages like this already.

LMK what's the best way to proceed. But again, my feeling is that mutateElement() gives us the right place/tools to deal with hidden/unhidden case here.

[torch2424]

@dvoytenko Ahh that makes sense. Yes, if that is the case, then perhaps a simple hidden attribute observer shouldn't apply to this case. Sounds like AMP subscriptions should be an exception to this, and indeed use Resources.mutateElement() to track complex visibility states.

@lannka @jridgewell I think we can proceed to fix this using Resources.mutateElement() for subscription-like use cases, and just maintain a list of AMP element tags that should follow this exception. What do you think?

[torch2424]

After talking with @lannka , we thought it would be good to have Resources.mutateElement() dispatch an event that analytics can listen for, and then act accordingly.

We both agreed a hidden attribute mutation observer isn't the best route for this, and listening to an event would be a good idea. Though, I am wary to increase the runtime size for this. Any better suggestions would be great appreciated before we start an implementation.

[dvoytenko]

I think the event from mutateElement is a great idea! I doubt this will materially add anything to the runtime size.

[torch2424]

@dvoytenko Awesome, we'll go ahead and implement using this plan once I get the chance. Thanks! 😄

[torch2424]

@gremz

I went ahead and made an example to start investigating this issue: https://github.com/torch2424/amphtml/blob/19030-amp-access-analytics-on-visible/test/manual/amp-access-analytics.html

And I noticed that everything worked fine:

And after looking at your example, it seems that the image you are providing as the paywall doesn't have a defined width and height attribute? Therefore, I don't think AMP runtime is laying it out, thus not making it visible. So when you scroll, perhaps its a different bug that is then calling the is visible when it is not?

cc @dvoytenko for their thoughts on this

[torch2424]

Pinging @gremz again 😄

I'll wait a bit longer before closing as "could not reproduce". We can still chat on the issue if closed however.

[gremz]

@torch2424 sorry for the delay. It looks the example you provided doesn't have the amp-access-hide attribute on the <section amp-access="NOT subscriber"> element, without the attribute the trigger works since it is visible by default. In your example if subscriber were false I believe it would still trigger the event (w/ a flash of the paywall element).

[torch2424]

@gremz Thank you for the response! Sorry I missed that, I'll go ahead and re-investigate when I can 😄

[gremz]

Hi @torch2424, just wanted to check in to see if this ticket is anywhere on your roadmap. Thanks!

[torch2424]

@gremz Thanks for your patience! Q1 was very busy for us, and lost track of this.

cc @jridgewell @dvoytenko Do either of you know someone who would have bandwidth on this? I can keep this in my backlog still, but if you know someone willing to pick this up sooner than I can, It would be appreciated 😄

[gremz]

@torch2424 I noticed this issue stopped occurring around 11/14/2019. Any chance some sort of release occurred around this time? If so, would you have any idea what fixed this? Thank you.

[torch2424]

@gremz Glad to hear the issue got fixes! So I am no longer on the AMP team at Google, so I wouldn't know why / or have access to why it may have gotten fix. You may be able to get some help from @jridgewell or @dvoytenko on why it stopped occurring 😄

Hope that helps! Thanks!