Fix social media link previews on static pages#3185
Open
Conversation
…re rendered for consistency across different clients
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When sharing Sefaria static pages (like
/jobs,/team,/educators,/about) on social media platforms and messaging apps (LinkedIn, Facebook, Slack, iMessage), the link preview description would sometimes display "Skip to main content" instead of the page's actual description. The link preview title could also appear blank.This happened because the OpenGraph and Twitter meta tags (
og:description,og:title,twitter:description,twitter:title) were empty for static pages. When social media scrapers encounter empty OpenGraph tags, they fall back to extracting the first visible text from the page body or, if none is present, the page title. That first text could either be blank or come from a React accessibility skip link rendered during server-side rendering ("Skip to main content").The root cause was an architectural mismatch in
base.html. The regular<meta name="description">tag and the OpenGraph<meta property="og:description">tag were controlled by separate, independent Django template blocks ({% block description %}and{% block fb_description %}). Static page templates only overrode{% block description %}, leaving the OpenGraph block to fall back to a Python variable (desc) that theserve_staticview never set. The same problem affected titles —og:titleandtwitter:titleused the Python variabletitledirectly without any block mechanism at all.A secondary bug was also present: the
og:descriptiontag was nested inside{% block ogimage %}. Several visualization pages (/explore,/visualize/library, etc.) overrode this block to set a custom preview image, which inadvertently removed theirog:descriptiontag entirely.Design Decisions
Why custom template tags instead of a capture variable
I considered two approaches:
Capture approach: Add a
{% capture as page_desc %}custom tag that renders a block's output into a variable, then reuse that variable for all three meta tags. This would require zero changes to static page templates but introduces indirection. Future developers (or my coding agent friends) would need to understand that the OG description is fed by a captured variable from a different block.Grouped block approach (chosen): Create
{% meta_title %}and{% meta_desc %}template tags that each take a single content value and output all three synchronized meta tags (regular, OpenGraph, and Twitter). Group these inside{% block meta_title %}and{% block meta_description %}blocks that child templates can override.I chose the grouped block approach because:
ogimagesecondary bug. Movingog:descriptionout of{% block ogimage %}means visualization pages that overrideogimagefor a custom image no longer accidentally lose their description.{% block meta_description %}{% meta_desc %}Your description here{% endmeta_desc %}{% endblock %}— one block, one concept.The trade-off is that this approach requires updating many child templates.
Why block-style tags instead of simple tags
The custom tags use a block-style syntax (
{% meta_desc %}...{% endmeta_desc %}) rather than a simple argument syntax ({% meta_desc "some text" %}). This allows arbitrary Django template expressions inside the tag, including{% trans %},{% blocktrans %},{% if %}conditionals, and variable references like{{ desc|striptags }}, without needing first to capture the value into a variable, without needing first to capture the value into a variable.Code Changes
Custom template tags (
reader/templatetags/sefaria_tags.py)Added two new template tags:
{% meta_title %}...{% endmeta_title %}— renders its content into synchronized<title>,<meta property="og:title">, and<meta name="twitter:title">tags.{% meta_desc %}...{% endmeta_desc %}— renders its content into synchronized<meta name="description">,<meta property="og:description">, and<meta name="twitter:description">tags.Both tags use
conditional_escapeto safely handle special characters in the content, preventing potential XSS if descriptions contain quotes or HTML characters.Base template (
templates/base.html){% block title %},{% block description %},{% block fb_description %},{% block soc_description %}) with two grouped blocks:{% block meta_title %}and{% block meta_description %}.og:descriptionout of{% block ogimage %}so that templates overriding the image block no longer accidentally drop their description.og:titleandtwitter:titletags that previously used the Python variable directly with no block override mechanism.Child templates (with live routes)
Updated all templates that extend
base.htmlto use the new block names and custom tags.Notes
/Genesis.1, topic pages, profile pages) are unaffected. Their views passdescandtitleas Python context variables, which the base template's default{% meta_desc %}{{ desc|striptags }}{% endmeta_desc %}handles correctly.templates/edit_text.htmlhas its own standalone<meta name="description">but does not extendbase.html, so it is not affected by these changes.