Skip to content

fix(discourse): don't match a topic when the title is empty - #3065

Open
dj-jarvis wants to merge 1 commit into
MetaMask:mainfrom
dj-jarvis:fix/discourse-empty-title-false-match
Open

dj-jarvis wants to merge 1 commit into
MetaMask:mainfrom
dj-jarvis:fix/discourse-empty-title-false-match

Conversation

@dj-jarvis

Copy link
Copy Markdown

Don't match a Discourse topic when the page title is empty

Problem

In src/components/DiscourseComment/index.jsx, normalizeTitle coerces a missing title to an empty string:

const normalizeTitle = str =>
  str?.toLowerCase().replace(/[|:]/g, ' ').replace(/\s+/g, ' ').trim() || ''

That value then feeds the topic-matching comparison:

const titleMatch =
  wordOverlap > 0.5 ||
  topicTitleNorm.includes(metaTitleNorm) ||
  metaTitleNorm.includes(topicTitleNorm)

In JavaScript, String.prototype.includes('') always returns true — the empty string is a substring of everything.

Impact

  • When metadata.title normalizes to '', metaTitleNorm.includes(topicTitleNorm) is true for any topic title. Combined with categoryMatch, .find() returns the first topic in the category — an arbitrary discussion unrelated to the page.
  • The resulting topicId is passed to the Discourse embed, so another tutorial's comments render under this page. Readers can mistake them for a discussion of this material.
  • The failure is silent: no console output, no build error, no test failure — it just returns a "successful" but wrong topic id.

The wordOverlap > 0.5 guard does not prevent this. With empty titles both word sets are empty, so wordOverlap is forced to 0 and the includes check alone decides the outcome.

title is optional in Docusaurus frontmatter, so frontMatter.title is undefined whenever an author omits it. Even a present title can normalize to '' if it consists solely of |, : or whitespace, since normalizeTitle strips exactly those characters.

Fix

Require both normalized titles to be non-empty before comparing:

const titleMatch =
  !!topicTitleNorm &&
  !!metaTitleNorm &&
  (wordOverlap > 0.5 ||
    topicTitleNorm.includes(metaTitleNorm) ||
    metaTitleNorm.includes(topicTitleNorm))

The guard sits ahead of the whole expression, so wordOverlap > 0.5 keeps its previous behaviour for non-empty titles while the empty-string false positive is removed.

@dj-jarvis
dj-jarvis requested review from a team as code owners October 3, 2026 21:31
@vercel

vercel Bot commented Oct 3, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Consensys Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant