immich-app / immich

High performance self-hosted photo and video management solution.
https://immich.app
GNU Affero General Public License v3.0
41.44k stars 2.04k forks source link

Wrong photo count in share pages, duplicated <meta> tags #8605

Closed codeceptsDE closed 3 months ago

codeceptsDE commented 3 months ago

The bug

When sharing an album via link, the (opengraph/...) preview displays "0 shared photos & videos" as description, irrespective of the actual count. The wrong description is gathered from the duplicated <meta> tags in the share page's <head>. There, the <meta name="description", <meta propterty="og: and <meta name=twitter: tags appear twice: once with count 0, and at the end with the correct count. In fact, the <meta name="description" even appears a third time. The displayed count in the actual page body is correct.

The OS that Immich Server is running on

Docker on Debian

Version of Immich Server

v1.101.0

Version of Immich Mobile App

-

Platform with the issue

Your docker-compose.yml content

-

Your .env content

-

Reproduction steps

1. Create an album with some pictures in it
2. Create and copy a public share link
3a. Share it using an app that digests Open Graph Meta Tags (Signal, WhatsApp, ...)
3b. or visit this link and check the <head>

Additional information

The initially sent <head> looks like this, and (only) contains the description mentioning 0 photos:

initial header ``` ```

After all scripts have run, the following lines get appended at the end (whitespace by me). They do contain the correct count.

additional html header content after scripts ``` link rel="modulepreload" as="script" crossorigin="" href="https://redacted.domain/_app/immutable/chunks/mdi.BV22mFDw.js"> Test Album ```

The relevant sections are:

before scripts (beginning of header):

    <!-- (used for SSR) -->

    <meta name="description" content="0 shared photos &amp; videos">

    <!-- Facebook Meta Tags -->
    <meta property="og:type" content="website">
    <meta property="og:title" content="Test Album">
    <meta property="og:description" content="0 shared photos &amp; videos">
    <meta property="og:image" content="/api/asset/thumbnail/redacted-hash-1?key=redacted-key-1">

    <!-- Twitter Meta Tags -->
    <meta name="twitter:card" content="summary_large_image">
    <meta name="twitter:title" content="Test Album">
    <meta name="twitter:description" content="0 shared photos &amp; videos">

    <meta name="twitter:image" content="/api/asset/thumbnail/redacted-hash-1?key=redacted-key-1">

and after (end of header):

  <meta name="description" content="5 shared photos &amp; videos.">
  <meta property="og:type" content="website">
  <meta property="og:title" content="Test Album">
  <meta property="og:description" content="5 shared photos &amp; videos.">
  <meta property="og:image" content="/api/asset/thumbnail/redacted-hash-1?format=WEBP">
  <meta name="twitter:card" content="summary_large_image">
  <meta name="twitter:title" content="Test Album">
  <meta name="twitter:description" content="5 shared photos &amp; videos.">
  <meta name="twitter:image" content="/api/asset/thumbnail/redacted-hash-1?format=WEBP">
  <meta name="description" content="5 shared photos &amp; videos.">

Note how <meta name="description" even appears a third time.

Thoughts on a possible fix

I am unsure how different apps deal with metadata information and js. I suspect it may not be enough if the metadata tags are replaced once the scripts have loaded (instead of accidentally duplicating), because not all apps may wait until js has run. But if it's possible to load the thumbnail on the first request, so should be the photo count, I would assume.

nghduc97 commented 3 months ago

The issue seems to be caused by this pull request https://github.com/immich-app/immich/pull/5635 adding incorrect to meta tags into the document. The meta tags in web/src/routes/+layout.svelte seems to have existed before above PR.

Can any maintainers provide some insight if it is still needed? Since the solution seem to be just reverting it.

jrasm91 commented 3 months ago

The meta tags need to come in the raw html response itself, directly from the server. Link unfurlers don't always run a web browser, execute javascript, and wait for the page to render before looking for the tags. They are added in svelte client-side, which is the reason why they were added server-side instead.

nghduc97 commented 3 months ago

The meta tags need to come in the raw html response itself, directly from the server. Link unfurlers don't always run a web browser, execute javascript, and wait for the page to render before looking for the tags. They are added in svelte client-side, which is the reason why they were added server-side instead.

Oh I see, I thought the server uses Svelte SSR

jrasm91 commented 3 months ago

Yeah, we currently bundle the svelte app as a static build in the immich-server container.

nghduc97 commented 3 months ago

Wrong asset count aside, do you think we should de-duplicate the meta tags?

Currently the locally run server (npm run dev) only display client renderred meta tags. It can be modified to render only if SSR tags don't already exist.

jrasm91 commented 3 months ago

I don't think we need the client-side tags at all actually. We can probably comment them out.

jrasm91 commented 3 months ago

The only caveat here is we might change from the static adapter to the node adapter and use the exported handler in the future. In that case we would no longer need the custom server implementation and the current svelte implementation would work as expected.

https://kit.svelte.dev/docs/adapter-node#custom-server