r-lib / pkgdown

Generate static html documentation for an R package
https://pkgdown.r-lib.org/
Other
721 stars 336 forks source link

Incorrect site root path for packages in dev status? #2814

Closed jayhesselberth closed 16 hours ago

jayhesselberth commented 2 weeks ago

In trying to fix #2804, I think I uncovered a different issue. It doesn't look like the site root path is set correctly for packages in dev status.

The paths to the head assets (favicon, JS, CSS) in the template are specified like this:

https://github.com/r-lib/pkgdown/blob/c40ba2c989e786bbb2829b431114d26375bbe19a/inst/BS5/templates/head.html#L8

BUT in a site in dev status (i.e., pkgdown), site.root is an empty string. I think it should actually be /dev/ or dev/.

Here's the relevant bit of pkgdown::data_template() run on current pkgdown:

site:
  root: ''
  title: pkgdown

So what ends up happening on a dev site is that the site is copied to /dev, but the href values in the site head don't have the /dev/ prefix.

I am guessing this has gone unnoticed because most packages start with a release version, where everything (assets and site) is copied to /. Then, in a subsequent dev version, the site contents are copied to /dev, but the assets are still in /, so the head href links still work, but they're using the assets from the released site in /.

This is also why the favicon checker is failing in #2804. The head of the deployed site has:

<link rel="icon" type="”image/svg+xml”" href="favicon.svg">

but the file is actually in /dev/favicon.svg (correct link). And the deployed site only has the stuff in /dev and can't rely on anything in /.

You can also see this in the dev pkgdown site. It's head has the following, so is using the favicon assets from the released site:

<link rel="icon" type="image/png" sizes="16x16" href="favicon-16x16.png">

I think the fix is to make site.root mirror pkg$development$prefix (but with a leading slash, so /dev/ instead of dev/)

jayhesselberth commented 1 day ago

@hadley does the above make sense? I'm struggling to get the href paths right but I think the issue is real.

hadley commented 1 day ago

Yeah, I think that makes sense.

jayhesselberth commented 16 hours ago

I'm closing this for now as this only impacts dev sites. It's also unclear how to specify the paths so that favicons work for non-root sites.