Fix bookmark favicon selection in oEmbed fallback

[lujuldotcom]

Hi @9larsons,

I tested the merged change and found that bookmark cards can still prefer Apple Touch Icons in the oEmbed fallback path.

The bookmark-specific favicon logic works when fetchBookmarkData is called with type === 'bookmark', but the fallback path currently calls it with the original undefined type:

if (!data && !type) {
    data = await this.fetchBookmarkData(url, body, type);
}

That means pickFn does not enter the bookmark-specific branch and falls back to the previous Apple-first behavior.

This change passes 'bookmark' explicitly in that fallback path:

data = await this.fetchBookmarkData(url, body, 'bookmark');

[coderabbitai]

Walkthrough

fetchOembedDataFromUrl now passes 'bookmark' explicitly to fetchBookmarkData when no oEmbed data is found and no type is provided. This changes the bookmark fallback so it uses a fixed bookmark card type instead of a falsy or undefined value.

Possibly related PRs

• TryGhost/Ghost#28768: Also touches the oEmbed-to-bookmark fallback path in oembed-service.js and related bookmark data handling.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: fixing bookmark favicon selection in the oEmbed fallback path.
Description check	✅ Passed	The description is directly related to the code change and explains the fallback type fix.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

ghost/core/core/server/services/oembed/oembed-service.js (1)

568-568: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression test for the undefined-type fallback.

This fixes the right contract, but the supplied tests only cover the two halves separately (fetchOembedDataFromUrl() returning bookmark data, and fetchBookmarkData(..., 'bookmark') preferring the standard favicon). A single test for the exact !type fallback path would lock this bug down.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/core/server/services/oembed/oembed-service.js` at line 568, Add a
regression test for the exact undefined-type fallback path in oembed-service.js:
the current coverage splits the behavior across fetchOembedDataFromUrl() and
fetchBookmarkData(), but it does not verify the branch where !type triggers
fetchBookmarkData(url, body, 'bookmark'). Add a focused test around
OEmbedService’s fallback logic to assert that an undefined oEmbed type resolves
to bookmark data and still uses the standard favicon behavior, so this specific
contract is locked down.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ghost/core/core/server/services/oembed/oembed-service.js`:
- Line 568: Add a regression test for the exact undefined-type fallback path in
oembed-service.js: the current coverage splits the behavior across
fetchOembedDataFromUrl() and fetchBookmarkData(), but it does not verify the
branch where !type triggers fetchBookmarkData(url, body, 'bookmark'). Add a
focused test around OEmbedService’s fallback logic to assert that an undefined
oEmbed type resolves to bookmark data and still uses the standard favicon
behavior, so this specific contract is locked down.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 34c66b35-e43d-46e9-a29d-96dc5c1d1a14

📥 Commits

Reviewing files that changed from the base of the PR and between 0ed0d9f and d30a80e.

📒 Files selected for processing (1)

• ghost/core/core/server/services/oembed/oembed-service.js