Page MenuHomePhabricator

Remove LinkRenderer feature for Skin footer link (and remove wgFooterLinkCacheExpiry)
Closed, ResolvedPublic

Description

What

  • Skin::preloadExistence for footer links - Introduced in 2016 with change 314799 by me to reduce database queries on every page view.
  • $wgFooterLinkCacheExpiry - Introduced in 2020 with change 582611 by @aaron, but currently unused (disabled by default and at WMF).

Context

[…]

When I last refactored this code, I considered proposing to remove this feature. Instead, I first optimised it so that it would no longer be a perf concern, and then forgot about it.

Removing it would mean the footer links behave (very slightly) differently. In particular, it means that they no longer get a red color if the target page doesn't exist for any reason. The downside of this would be that on a brand new wiki, it would be less obvious to the community that some of their default project pages have yet to be created. Another scenario is if a localisation update or interface admin changes the message that decides where the link goes to, and if the new target doesn't exist. It'd be nice to easily spot that through a red link. How often does someone with knowledge of how to fix it or report it, click on footer links to see if they still work?

On the other hand, we don't provide this affordance to sidebar links (although perhaps we should?) Either way, the sidebar has its own batch loader and cache, so it wouldn't be a performance issue there even if we did support red links there. Perhaps this would be a useful thing to surface [..] for admins, [..]

Proposal

Make the footer links always blue and without applying LinkRenderer features adding a class to the element for "stub threshold" and for "is redirect".

Rationale

  • Contrary to my context comment above, the footer links are already always blue. When I observed the LinkRenderer database queries, I assumed these queries result in at least somewhat-useful behaviour such as making links red or blue. However the skin has, since the introduction of the footer links in ~2004, always called Linker->makeKnown and so the database metadata is never used for that purpose. It never gets class=new and thus never becomes red.
  • The metadata feature for stub threshold was removed from MW core.
  • The metadata feature for redirect status results in a class mw-redirect being added, which has no effect by default and I can't think of a even a theoretical use case where this is important to surface in the footer. At least redlink would be somewhat useful to surface to site admins (though not on every page view per-se).

Event Timeline

Change 815836 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] Skin: Skip LinkRenderer in footerLink and remove $wgFooterLinkCacheExpiry

https://gerrit.wikimedia.org/r/815836

Change 815836 merged by jenkins-bot:

[mediawiki/core@master] Skin: Skip LinkRenderer in footerLink and remove $wgFooterLinkCacheExpiry

https://gerrit.wikimedia.org/r/815836

Krinkle claimed this task.
Krinkle triaged this task as Medium priority.

$wgFooterLinkCacheExpiry is now completely unused, but not even marked as deprecated. Was the intention to remove the setting itself also part of this task?

Change 819498 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] Skin: Remove schema leftover for removed $wgFooterLinkCacheExpiry

https://gerrit.wikimedia.org/r/819498

@Mainframe98 The setting represented an optimisation with either no visible change in behaviour or a reduction in guruantees (e.g. destination changes delayed by cache). Removing that without performance impact does not a breaking change as as it simply means the logic is now even faster and no longer subject to any delay. The existence of the setting itself in any LocalSettings.php files, unlike e.g. regular runtime interaction such as calling deprecated methods and reading deprecated variables, is also fine as it does not affect the runtime whether an additional variable is declared that isn't used.

As for the config variable in the main schema in core, that was meant to be removed in my patch, which I forgot!

Change 819498 merged by jenkins-bot:

[mediawiki/core@master] Skin: Remove schema leftover for removed $wgFooterLinkCacheExpiry

https://gerrit.wikimedia.org/r/819498

Change 890872 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] linker: Remove DB lookups from makeKnownLink() and improve docs

https://gerrit.wikimedia.org/r/890872