Skip to content
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

AO3-6702 Prerender all links on hover #4774

Closed
wants to merge 1 commit into from

Conversation

domenic
Copy link
Contributor

@domenic domenic commented Mar 29, 2024

Pull Request Checklist

Issue

https://otwarchive.atlassian.net/browse/AO3-6702

Purpose

Adds prerendering of any link on hover, in addition to the idle-time prerendering of the next chapter. This makes clicking around Ao3 pretty snappy in general!

Testing Instructions

See https://otwarchive.atlassian.net/browse/AO3-6702

References

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type/speculationrules explains the new "source": "document" syntax we're using.

Credit

Domenic Denicola, he/him

"source": "document",
"eagerness": "moderate",
"where": {
"href_matches": "/*"
Copy link
Contributor

@de3sw2aq1 de3sw2aq1 Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with prefetching, but it looks like you need to exclude unsafe URLs such as those with side effects: https://developer.mozilla.org/en-US/docs/Web/API/Speculation_Rules_API#unsafe_prefetching

A number of "buttons" on ao3 are plain <a> links with an href causing side effects, like GET /works/:id/mark_for_later. I think there would be issues allowing prerender of these links.

"href_matches": "/*" is probably unsafe

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, there are a lot of actions that are accessed via GET requests: https://otwarchive.atlassian.net/browse/AO3-5953

@Bilka2
Copy link
Contributor

Bilka2 commented Apr 11, 2024

It appears very unsafe to me to prerender every link hovered. Based on https://developer.mozilla.org/en-US/docs/Web/API/Speculation_Rules_API#unsafe_prerendering, prerendering is more risky than prefetching, so the entire website would have to be vetted to not do any of the mentioned problematic things. For example, it's unclear to me how the prerendering interacts with the current cookie based redirection. As already mentioned above, there are also links that do unsafe get requests.

There are also userscripts that may interact badly with prerendering but it's difficult for me to determine.

Furthermore, there are a lot of links on AO3 that may be hovered (accidentally) but aren't likely to be clicked, like for example all the tag links in work blurbs in work listings. Prerendering these appears very overkill to me, at most prefetching may be sensible. But even then I'd say that targeted prefetching for example of the works in a work listing seems like a much better idea than simply scattershot prerendering everything hovered.

Generally I doubt the utility of spending bandwidth and server resources on sometimes speeding up loading. I'd expect that even the already implemented targeted approach of prerendering the next chapter is loading much more than users end up visiting. Both the strategy of opening a work and then leaving the tab in the background as well as opening something and then going back in the browser are common in AO3 culture. In all those cases, even the very obvious strategy of prerendering the next chapter is doing unnecessary work and consuming unnecessary resources. The tradeoffs may weigh on the side of prerendering for chapters, but most other paths users take on the website are even less straightforward than navigating through the chapters of a work. Though of course this is hard to judge without analytics.

To sum up, I think this PR should not be merged. I think it would be better only do targeted prefetching or preferably not add any more prerendering/prefetching at all.

@sarken
Copy link
Collaborator

sarken commented Apr 11, 2024

Sorry, I don't realize this was still open! We discussed this in Slack with Domenic shortly after it was submitted and decided we wanted to limit this to prerendering works on hover, pending some questions about how that interacts with reading history. I'll close this for now.

@sarken sarken closed this Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants