The converting of relative links to absolute in the footer component is problematic and surprising

[kevindew]

I'm raising this issue after fixing a bug in GOV.UK Chat where we finding that all our footer links were incorrect due to the changes introduced in https://github.com/alphagov/govuk_publishing_components/pull/3699. I also notice we're also not the first team to have had a problem with this: https://github.com/alphagov/content-publisher/pull/3149

The changes, applied to the layout footer component change the user input of the hrefs of links that are injected into the component to be absolute URLs with GOV.UK website root.

Why this causes a problem

The footer converts all relative links passed into it that are not absolute into absolute links for www.gov.uk. This causes a problem for any applications that use this component and are not served on www.gov.uk as they cannot use this component without providing their own absolute links.

Why this is a surprising

It is consistent with other components that do not manipulate hrefs passed into components. It also couples the component to a particular use case - www.gov.uk, whereas it should be isolated. It also uses an environment for GOV.UK Docker (VIRTUAL_HOST) which can conceal it's effects when a user is developing the application.

What should we do about it

It looks like this is solving a problem that was previously solved in a more elegant way, that CSV previews are hosted on the assets hostname and thus relative links don't work. We should see if we can apply the previous solution again so we can have relative links without problems. The previous solution was to use a base element with a href of GOV.UK website root : https://github.com/alphagov/whitehall/pull/5764 which allowed all relative links to work, this probably got missed moving the functionality out of Whitehall.

layout_footer should not convert data that is pushed into it, instead if it needs absolute URLs they are the parameters that should be pushed in as data into the component to be consistent with other components.

[AshGDS]

Thanks @kevindew - yep it isn't the best solution. For some more context, I believe initially I had left my PR as a draft for some time, as I was mindful that it was hacky and that it became a "frontend dev trying to implement a backend-y thing" scenario. However there was then some urgency to fix the issue on the CSV preview pages before everyone went away for Christmas, so the PR ended up getting pushed through to production.

For some more context, here's the original discussion:
https://gds.slack.com/archives/CARGH33JS/p1698941129453809

And the follow up which led this being merged: https://gds.slack.com/archives/C052X8S2P8A/p1702555513615439

And thanks for bringing attention to the <base> element solution. I can see that this would affect anchor links. We have a Skip to content link as the first focusable element on our pages for accessibility. This links to #content. I think if we used <base> this would make the anchor link https://www.gov.uk/#content? I guess we would possibly need to modify the skip link component to keep it functioning with a <base> tag present. And I guess would we need to note the existence of <base> if any other anchor links are added in the future on pages using this.

[kevindew]

Thanks for looking at this so quickly Ash - and no worries, I know how something broken can need immediate fixes.

That's a good shout about base affecting anchor elements, we may have even missed that on the implementation of this last time. That's definitely an undesirable consequence and seems a battle to fix 🤔. Interesting as well as the main advantage of base element is not having to play whack-a-mole trying to catch every relative link, but with that you have to play a different whack-a-mole to catch every relative anchor 🤦 .

If we rule out that approach then, maybe the absolute URL code is better placed in here: https://github.com/alphagov/govuk_publishing_components/blob/3f51ed9f9a00706a08772db18ff577c01346daf6/lib/govuk_publishing_components/presenters/public_layout_helper.rb#L49-L50 where it ends up entirely specific for the public layout component and can be injected into the layout_footer component as absolute URIs?

[AshGDS]

Thanks @kevindew - yep the <base> approach seemed great except for that anchor issue 😅. Good idea moving it into the public layout, that approach seems like it will work.

Our next triage meeting is September 12th, so unless someone else is better suited to picking up the card, I can assign myself to this when it pops up in our triage meeting - as long as I'm not stretched too thin!