WICG / soft-navigations

Heuristics to detect Single Page Apps soft navigations
https://wicg.github.io/soft-navigations/
Other
46 stars 6 forks source link

DOM changes which are non-user visible are still eligible for soft navigations #18

Closed Smrtnyk closed 11 months ago

Smrtnyk commented 1 year ago

Currently described heuristics do not specify whether the soft navigation entry should be triggered if there is a click on the anchor element that maps to some element with specific id on the page. Such click would just cause scroll to the element. For example, click on an anchor element with href of #some-id on the page would case browser just to scroll down to that element. Currently in chrome canary aforementioned action causes a soft navigation entry. Is that expected?

tunetheweb commented 1 year ago

A soft nav should require a DOM change. So an anchor element click by itself will not cause a soft navigation entry.

See this example here: https://www.tunetheweb.com/experiments/softnavsdemo/index2.html

Is your anchor link causing the DOM to change (for example by loading some lazy-loaded content).

Smrtnyk commented 1 year ago

I tried it on https://vuejs.org/sponsor/ by clicking on some of the headers which is linked as an anchor element and just scrolls to some section. On chrome canary it triggered a soft navigation.

tunetheweb commented 1 year ago

Interesting. That does result in a DOM change as it inserts an image temporarily (for tracking?)

https://github.com/WICG/soft-navigations/assets/10931297/7d368606-c26e-4274-beec-49c78e2eca3b

So yes, with the current heuristics that counts as a soft navigation. But that seems less than ideal...

Smrtnyk commented 1 year ago

We have a RUM product, and currently we are wrapping history API to detect soft nav changes. We also explicitely filter out hashchange events in case anchor part corresponds to an id of some element on the page, so we don't treat scrolls as soft navs. I agree that this situation is not ideal in current vue page scenario

yoavweiss commented 1 year ago

Interesting. That does result in a DOM change as it inserts an image temporarily (for tracking?) So yes, with the current heuristics that counts as a soft navigation. But that seems less than ideal...

That's unfortunate indeed. Vue seem to be using https://usefathom.com/ as an analytics solution. It seems like it would be better for them to avoid injecting the image to the DOM, but rather than create a detached image, that would still beacon up their analytics data. Better yet, they use sendBeacon or fetch() with a keepalive flag, that are the platform primitives that tackle this use case.

^^ @JackEllis - for thoughts on the above.

We also explicitely filter out hashchange events in case anchor part corresponds to an id of some element on the page, so we don't treat scrolls as soft navs.

I don't think soft navigations should ignore hash changes, as there are many SPAs out there that are using hash changes (it preceded the history API, and seems to be a pattern still in use).

If this case is common and goes beyond a single library, we can look into adding some more complexity to the heuristics to be able to ignore this case.

Smrtnyk commented 1 year ago

I don't think soft navigations should ignore hash changes, as there are many SPAs out there that are using hash changes (it preceded the history API, and seems to be a pattern still in use).

I am not saying we are listening to hashchange just to ignore them all, I am saying we just filter out the ones that just cause scroll on the page. Seems like other bigger RUM vendors such as DataDog also agree with US (the company I work for that has a RUM product) https://github.com/DataDog/browser-sdk/blob/28cd180964e1b08853b967411d3473e7c00b79a5/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts#L280

It would be good to have somewhere written, by authority that created the spec, why would click on an anchor element, that just causes a scroll on the page, would cause soft navigation to happen. So that we (RUM vendors) can change our mind, and accept better standards. In case of Vue it is adding an image, but it can be caused by any other unrelated DOM mutation, and it would result in soft navigation.

If this case is common and goes beyond a single library, we can look into adding some more complexity to the heuristics to be able to ignore this case.

I feel that it has a potential to be a very common scenario.

yoavweiss commented 1 year ago

It would be good to have somewhere written, why would click on an anchor element, that just causes a scroll on the page, would cause soft navigation to happen. In case of Vue it is adding an image, but it can be caused by any other unrelated DOM mutation, and it would result in soft navigation.

This is not an "unrelated DOM mutation". It's a very much related DOM mutation, triggered as part of the hashchange event. You can see in the usefathom script that a hashChange event calls trackPageview() which calls send(), which is adding and the removing that image.

Maybe we can build heuristics that can distinguish this DOM addition from e.g. a hero image DOM addition. But that's a lot of complexity and there's no evidence at the moment that many other libraries are adding such (spurious) images to the DOM for tracking purposes.

Smrtnyk commented 1 year ago

This is not an "unrelated DOM mutation". It's a very much related DOM mutation, triggered as part of the hashchange event.

I see what you are trying to say, but my point is that it is irrelevant to the end user, who sees nothing on the page but a quick scroll to the heading. Image mutation is not even observed from a end user perspective. Thanks for responses. I will try the soft nav feature on some other pages and come back if I find it happening more often. Until then, I am fine if you want to close this issue.

yoavweiss commented 1 year ago

Image mutation is not even observed from a end user perspective.

I agree it is not observable by the user. It's just that it's hard to distinguish that from a very similar image addition to the DOM that is user visible.

Thanks for responses. I will try the soft nav feature on some other pages and come back if I find it happening more often.

Thanks for testing this! :)

Until then, I am fine if you want to close this issue.

We can keep it open in the meantime.

JackEllis commented 1 year ago

That's unfortunate indeed. Vue seem to be using https://usefathom.com/ as an analytics solution. It seems like it would be better for them to avoid injecting the image to the DOM, but rather than create a detached image, that would still beacon up their analytics data. Better yet, they use sendBeacon or fetch() with a keepalive flag, that are the platform primitives that tackle this use case.

^^ @JackEllis - for thoughts on the above.

Yup, we'll soon be moving away from the whole pixel insert solution and over to using POST requests for analytics requests.

tunetheweb commented 1 year ago

@Smrtnyk or @yoavweiss could you edit this issue to rename it to something like "DOM changes which are non-user visible are still eligible for soft navigations" or something like that? Went looking for this example recently and took me a bit to find it.

yoavweiss commented 1 year ago

This should be addressed in the implementation with the recent move to contentful paints, rather than simple DOM modifications as the criteria. I'm planning to update the explainer/spec shortly.

yoavweiss commented 11 months ago

Updated the spec and explainer to indicate we're relying on contentful paints.