Skip to content

feat(seo): fix VideoObject schema and add video sitemap - #2186

Merged
rudrakshi-gupta merged 8 commits into
developfrom
feature/1622-video-seo-sitemap
Oct 20, 2026
Merged

rudrakshi-gupta merged 8 commits into
developfrom
feature/1622-video-seo-sitemap

Conversation

@rudrakshi-gupta

@rudrakshi-gupta rudrakshi-gupta commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

GoDAM already adds VideoObject JSON-LD to every page with a GoDAM video, but the schema had problems:

  • It pointed Google at the DASH .mpd manifest, which Google can't index.
  • It had no embedUrl.
  • It output empty descriptions.
  • isFamilyFriendly could be stored as a string.

There was also no video sitemap.

This PR delivers v1 of the Video SEO epic:

  • It fixes the schema.
  • It adds a video sitemap that plugs into Yoast SEO, Rank Math, All in One SEO, or the WordPress core sitemap.
  • It adds a site-wide on/off switch (off by default) and a per-video "Exclude from video sitemap" toggle.
  • It warns editors when a video title looks like a raw filename.

No new public URLs are created. The sitemap lists only pages that already exist, which avoids the thin-page problem from #616 / #1798 / #1836.

Part of #1622 (v1). The key moments, transcript-in-schema and graph-merge items are left for the follow-up release; see Out of scope.

Before / after

Before After
contentUrl DASH .mpd (rtgodam_transcoded_url), which Google can't index HLS .m3u8, falling back to the original file. Never .mpd
embedUrl Missing GoDAM player URL (?godam_page=video-embed&id=<id>)
description Output as "" when empty Own description → transcript excerpt (160 chars) → page excerpt → omitted
isFamilyFriendly (bool) cast, so "false" became true Real JSON boolean; string values parsed correctly
uploadDate Output as "" when empty Omitted when empty
Filename-style titles No warning Editor warning in Gutenberg, Elementor and WPBakery
Video sitemap None /godam-video-sitemap.xml, listed in the active SEO plugin's index or in /wp-sitemap.xml

What's included

1. Schema fixes (inc/classes/class-seo.php)

  • contentUrl
    • New get_attachment_content_url() returns the HLS URL (rtgodam_get_hls_transcoded_url_from_attachment()), or the original file URL when no HLS URL exists. A DASH URL is never returned (is_dash_url()).
    • It's resolved at output time from attachment_id, so block pages cached before this change, whose post meta still holds the .mpd, are corrected without a re-save.
    • Older Elementor and WPBakery caches have no attachment_id, so they can't be resolved at output time. The first time such a page is output, it's queued for a one-time background re-save:
      • A post meta flag (_godam_seo_legacy_resave) makes sure each page is queued only once.
      • Page IDs are collected for the request and written to one queue option (godam_video_seo_resave_queue) at shutdown, with a single WP-Cron event (godam_video_seo_resave_batch).
      • Each cron run re-saves 10 pages and schedules the next run while pages remain, so a site with hundreds of old pages doesn't rewrite the cron option or re-save everything in one request.
    • Videos that finish HLS transcoding after the page was saved switch to HLS automatically.
    • For entries without an attachment, a stored .mpd is dropped rather than output.
  • embedUrl: new get_embed_url() builds the URL of GoDAM's existing video-embed page.
  • Description fallbacks (resolve_schema_description()):
    1. The entry's own description, with HTML stripped.
    2. The transcript excerpt (get_transcript_excerpt()).
      • It parses WebVTT and SRT (transcript_to_text()) and trims to 160 characters at a word boundary (trim_to_length()).
      • It's fetched only on save, then cached on the attachment in _godam_seo_transcript_excerpt, keyed by transcript path, so a new transcript is picked up.
      • Page renders only ever read the cache.
      • Files in uploads are read from disk; others go through wp_safe_remote_get() with a 3s timeout.
      • A failed fetch is cached too and retried after an hour, so a dead transcript URL doesn't slow down every save. Deleted transcripts are ignored.
      • Transcript paths with a NUL byte or outside uploads are rejected.
    3. The page excerpt, or the start of the page text with block comments and shortcodes stripped, without rendering.
    4. Otherwise the property is left out.
  • isFamilyFriendly: new Seo::to_bool() handles "True", "false", "0", "yes", "", etc.
  • Empty uploadDate is left out.
  • build_video_object_schema() is now public, so the sitemap reuses it. The sitemap and the schema always agree.
  • get_seo_from_attachment() gains an $allow_remote parameter, so the render-time collector never fetches a transcript.
  • Elementor and WPBakery entries now carry attachment_id, including override entries, so the same resolution applies to them.
  • The per-video exclude flag is carried into the cached entries as excludeFromSitemap.

2. Video sitemap (new inc/classes/class-video-sitemap.php, class-video-sitemap-core-provider.php)

  • URL: /godam-video-sitemap.xml, with pages 2+ at /godam-video-sitemap2.xml and so on. Sites on plain permalinks use /?godam_video_sitemap=N.

  • Content:

    • One <url> per published page that contains GoDAM videos.
    • One <video:video> per video, with thumbnail_loc, title, description, content_loc (HLS), player_loc, duration (seconds), publication_date, and family_friendly (no only when false).
    • Videos missing a required field are skipped. A missing thumbnail falls back to the featured image; a missing description falls back to the title.
    • The same video placed twice on a page is listed once.
  • Turned on or off by the setting, or in code with godam_video_sitemap_enabled (read at after_setup_theme, so a theme can use it).

  • Data source: the existing video SEO post meta (godam_video_seo_schema), passed through Seo::build_video_object_schema().

  • Exclusions:

    • Drafts, pending, scheduled and private posts (post_status = publish).
    • Password-protected posts.
    • Pages marked noindex by Yoast (per page and the post-type default), Rank Math (Helper::is_post_indexable()) or AIOSEO (per page and the post-type default).
    • Videos with "Exclude from video sitemap" on.
  • Serving:

    • GoDAM serves the XML itself on parse_request at priority 1, so Yoast, Rank Math, AIOSEO and core don't intercept the *-sitemap.xml URL.
    • It sends X-Robots-Tag: noindex, follow.
    • A page past the last one returns 404.
    • A Sitemap: line for it is added to /robots.txt (skipped when the site discourages search engines).
  • Registration:

    Active SEO plugin Hook
    Yoast SEO wpseo_sitemap_index_links
    Rank Math rank_math/sitemap/index
    All in One SEO aioseo_sitemap_indexes
    None of the above Video_Sitemap_Core_Provider registered on wp_sitemaps_init (listed in /wp-sitemap.xml)

    Why GoDAM serves the XML itself: WordPress core's sitemap renderer only supports loc, lastmod, changefreq and priority, so it can't output <video:*> tags. The three SEO plugins also each handle custom sitemap content differently. Serving one XML document and listing its URL in whichever index is active gives identical output in every setup. The core provider overrides get_sitemap_url() to point at GoDAM's sitemap.

  • Scale and caching:

    • Pages of 1,000 URLs (filterable).
    • Page XML, total count and lastmod are cached in transients, keyed by a version option (godam_video_sitemap_cache_version).
    • The version is bumped when video SEO is saved or cleared, on publish/unpublish of a page with videos, on deletion of such a page or a used video, on edit_attachment, and on settings changes.
  • Multisite: transients and queries are per site, so each sub-site has its own sitemap at its own URL.

3. Settings

  • GoDAM → Settings → SEO (new tab, pages/godam/components/tabs/SeoSettings/SeoSettings.jsx):
    • "Enable video sitemap" toggle, off by default, stored at rtgodam-settings[seo][enable_video_sitemap] with defaults and sanitization in class-settings.php.
    • When it's on, the tab shows the sitemap URL and which index lists it. The data is localized as godamVideoSitemap from class-pages.php.
  • Per-video "Exclude from video sitemap" toggle:
    • Gutenberg: new block attribute excludeFromVideoSitemap, set from the Video SEO modal. It works whether SEO override is on or off.
    • Elementor: new seo_exclude_from_sitemap switcher in the SEO Settings popover.
    • WPBakery: new seo_exclude_from_sitemap param in the SEO Settings tab.
    • Excluding a video removes it from the sitemap only. Its JSON-LD still outputs.

4. Filename-style title warning

  • New isFilenameLikeTitle() and getFileNameFromUrl() in assets/src/blocks/godam-player/utils/index.js.
  • A title with no spaces is flagged when it has two or more hyphens/underscores (product-demo_v2), is a camera or phone name (IMG_2041, PXL_20240115), or ends in a video extension (clip.mp4). One separator isn't enough (Spider-Man, Paris-2024).
  • A title that matches the video's file name (with or without extension) is flagged only when it also looks machine-made: no spaces, or a date or time in it (Screen Recording 2024-01-15 at 10.30.45). WordPress builds titles from the uploaded file name, so a well-named upload such as Product Demo.mp4 → "Product Demo" isn't flagged.
  • Gutenberg: a warning notice under Headline in the Video SEO modal. The wording depends on override state: "edit the title in the Media Library" when override is off, "use a descriptive title" when it's on.
  • Elementor: the Headline help text turns into an amber warning, and updates as the headline, override or video changes.
  • WPBakery: a warning box under "Override SEO". It checks the Media Library title when override is off and the Headline field when it's on.

5. Editor contentUrl defaults

  • In the Gutenberg block (edit.js, VideoSEOModal.js) and the Elementor widget script, the prefilled SEO contentUrl now prefers rtgodam_hls_transcoded_url / hls_url over the DASH URL.
  • The help text no longer suggests MPD.
  • Player sources are unchanged.

New hooks

Hook Type Purpose
godam_video_seo_content_url filter Change the resolved contentUrl / content_loc for an attachment
godam_video_seo_embed_url filter Change or remove embedUrl / player_loc (return '' to omit)
godam_video_seo_description filter Change the resolved description
godam_video_sitemap_enabled filter Force the sitemap on or off in code
godam_video_sitemap_entries_per_page filter URLs per sitemap page (default 1000, max 50000)
godam_video_sitemap_post_types filter Post types that can appear (default: public types minus attachment)
godam_video_sitemap_exclude_post filter Exclude a page (receives the noindex result)
godam_video_sitemap_entry filter Modify or drop a page entry

Data changes

  • New attachment meta _godam_seo_transcript_excerpt: { path, text } cache of the transcript excerpt, or { path, failed_at } after a failed fetch.
  • New post meta _godam_seo_legacy_resave: set once a page with an older Elementor/WPBakery cache is queued for its re-save.
  • New option godam_video_seo_resave_queue: page IDs waiting for that re-save, not autoloaded; empty once the queue is done.
  • New WP-Cron event godam_video_seo_resave_batch: single events, only while the queue has pages.
  • New option godam_video_sitemap_cache_version: not autoloaded.
  • New setting rtgodam-settings[seo][enable_video_sitemap]: default false.
  • New block attribute excludeFromVideoSitemap: default false, so existing blocks need no migration.
  • No database schema changes, no rewrite-rule changes, no new public URLs.

Out of scope (follow-up release)

  • Key moments from chapters as Clip in hasPart (Add key moments into video SEO automatically from video chapters data #1597).
  • Transcript in the schema.
  • Listing videos that live only in templates, template parts or synced patterns in the sitemap. Their schema gets every fix here through the shared builder; they just aren't in post meta, so the sitemap can't list them yet.
  • Merging GoDAM's VideoObject into the Yoast / Rank Math schema graph.
  • Turning the sitemap on by default.

Testing

Automated

  • composer phpunit: all tests pass, including the new suites:
    • tests/php/VideoSeoSchemaTest.php (49): HLS contentUrl and fallbacks, .mpd never output, embedUrl, the description fallback chain and omission, transcript fetch/cache/parse (VTT and SRT), isFamilyFriendly booleans, empty uploadDate.
    • tests/php/VideoSitemapTest.php (39): well-formed XML and namespaces, escaping, required fields and fallbacks, sitemap/schema parity, de-duplication, every exclusion rule (status, password, Yoast/Rank Math noindex, per-video exclude, filter), URL parsing, pagination URL, off by default, duration conversion.
    • New stubs in tests/stubs/video-seo-functions.php.
  • npx wp-scripts test-unit-js assets/src/blocks/godam-player/utils/index.test.js: 35 tests pass (filename-title rule).
  • vendor/bin/phpcs on the changed PHP files: clean.
  • npm run lint:godam-hooks: balance OK, 0 uncovered call sites. New host-post reads are annotated with godam-coverage-ignore.
  • wp-scripts lint-js on the changed JS: no new errors.

Manual (all passed locally)

  1. Schema: on pages using the Gutenberg block, Elementor widget and WPBakery element, with SEO override on and off:
    • contentUrl is .m3u8, or .mp4 for untranscoded videos;
    • embedUrl loads the player;
    • description follows the fallback chain and is never "";
    • isFamilyFriendly is a JSON boolean;
    • Google Rich Results Test (Code tab) detects a Video with no errors.
  2. Old pages: a block page saved before this branch shows .m3u8 without a re-save. Elementor and WPBakery pages show it after one WP-Cron run.
  3. Template / synced pattern video: the footer schema has the same fixes.
  4. Filename warning:
    • appears for product-demo_v2 in all three editors;
    • clears with a descriptive title;
    • doesn't appear for How to set up GoDAM or Product demo - part 2.
  5. Settings:
    • the new SEO tab;
    • the sitemap is off by default;
    • /godam-video-sitemap.xml 404s while off;
    • the per-video toggle exists in all three editors.
  6. Sitemap:
    • correct entries and fields;
    • draft, private, password-protected and noindexed pages are excluded;
    • excluded videos are dropped while their schema remains;
    • listed in the Yoast, Rank Math and AIOSEO indexes and in /wp-sitemap.xml with no SEO plugin;
    • pagination via godam_video_sitemap_entries_per_page;
    • updates after a media title edit.
  7. Regression:
    • playback unchanged;
    • other settings tabs save;
    • the GoDAM for Woo Shoppable Video schema still outputs.

Screenshots

Screenshot 2026-10-14 at 3 02 46 PM Screenshot 2026-10-14 at 3 03 34 PM Screenshot 2026-10-14 at 3 04 08 PM Screenshot 2026-10-14 at 3 04 32 PM

Note for docs

Google shows a video as a video result only when the video is the main content of its page. The sitemap helps Google find videos; it doesn't make every page with a video appear as a video result.

Make GoDAM's video schema indexable by Google and add a video sitemap
that plugs into Yoast SEO, Rank Math, All in One SEO or the WordPress
core sitemap. No new public URLs are created.

- Schema contentUrl now uses the HLS (.m3u8) stream, falling back to the
  original file; the DASH .mpd is never output. Resolved at output time,
  so pages cached before this change are fixed without a re-save.
- Add embedUrl pointing at GoDAM's existing video-embed player page.
- Never output an empty description: fall back to a 160-char transcript
  excerpt (fetched on save only, cached in _godam_seo_transcript_excerpt),
  then the page excerpt, otherwise omit the property.
- Output isFamilyFriendly as a real JSON boolean (string values such as
  "false" are parsed correctly) and omit an empty uploadDate.
- Add Video_Sitemap: /godam-video-sitemap.xml with one <url> per
  published page and one <video:video> per video, built from the cached
  video SEO post meta via the shared schema builder so both always agree.
- Exclude drafts, private and password-protected posts, pages noindexed
  in Yoast / Rank Math / AIOSEO, and videos the editor excludes.
- List the sitemap in the Yoast, Rank Math and AIOSEO indexes, or via a
  core sitemap provider when none is active; paginate at 1000 URLs,
  cache per site with version-based invalidation.
- Add GoDAM > Settings > SEO with an "Enable video sitemap" toggle (off
  by default) and a per-video "Exclude from video sitemap" toggle in
  Gutenberg, Elementor and WPBakery.
- Warn editors in all three editors when a video title looks like a
  raw file name.
- Prefer the HLS URL when prefilling SEO contentUrl in the editors.
- Add new filters: godam_video_seo_content_url, godam_video_seo_embed_url,
  godam_video_seo_description, godam_video_sitemap_enabled,
  godam_video_sitemap_entries_per_page, godam_video_sitemap_post_types,
  godam_video_sitemap_exclude_post, godam_video_sitemap_entry.
- Add PHPUnit coverage (VideoSeoSchemaTest, VideoSitemapTest) and Jest
  tests for the filename-title check.
@rudrakshi-gupta rudrakshi-gupta self-assigned this Oct 14, 2026
Copilot AI balanced review requested due to automatic review settings October 14, 2026 09:36
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
15 0 15

⚠️ Warnings (15)

📁 inc/classes/class-video-sitemap.php (1 warning)
📍 Line 🔖 Check 💬 Message
620 WordPress.DB.SlowDBQuery.slow_db_query_meta_key Detected usage of meta_key, possible slow query.
📁 readme.txt (2 warnings)
📍 Line 🔖 Check 💬 Message
0 mismatched_plugin_name Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0 trademarked_term The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.
📁 inc/classes/class-video-sitemap-core-provider.php (1 warning)
📍 Line 🔖 Check 💬 Message
73 WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound Hook names invoked by a theme/plugin should start with the theme/plugin prefix. Found: "wp_sitemaps_index_entry".
📁 assets/build/css/main.css (1 warning)
📍 Line 🔖 Check 💬 Message
0 EnqueuedStylesScope This style is being loaded in all contexts.
📁 assets/src/libs/analytics.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/20/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
📁 assets/build/js/main.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/20/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Path containment, transcript parsing, sitemap cache invalidation, and title-length issues remain unresolved.

4 open findings
What changed in this PR

Adds corrected VideoObject metadata, a configurable video sitemap, and editor-side SEO guidance.

Changes:

  • Resolves indexable video URLs, descriptions, embeds, and boolean schema values.
  • Integrates a cached video sitemap with SEO plugins and WordPress core.
  • Adds sitemap controls, filename warnings, and automated tests.
File Description
tests/​stubs/​video-seo-functions.php Adds WordPress/SEO test stubs.
tests/​php/​VideoSitemapTest.php Tests sitemap behavior.
tests/​php/​VideoSeoSchemaTest.php Tests schema and transcripts.
pages/​godam/​redux/​slice/​media-settings.js Adds SEO settings state.
pages/​godam/​components/​tabs/​SeoSettings/​SeoSettings.jsx Adds SEO settings UI.
pages/​godam/​App.js Registers the SEO tab.
inc/​classes/​wpbakery-elements/​class-wpb-godam-video.php Adds sitemap exclusion control.
inc/​classes/​rest-api/​class-settings.php Sanitizes sitemap settings.
inc/​classes/​elementor-widgets/​class-godam-video.php Adds Elementor SEO controls.
inc/​classes/​class-video-sitemap.php Implements sitemap generation and integrations.
inc/​classes/​class-video-sitemap-core-provider.php Adds the core sitemap provider.
inc/​classes/​class-seo.php Corrects schema and transcript fallbacks.
inc/​classes/​class-plugin.php Initializes the sitemap service.
inc/​classes/​class-pages.php Localizes sitemap details.
assets/​src/​js/​wpbakery/​wpbakery-video-selector-param.js Adds WPBakery title warnings.
assets/​src/​js/​elementor/​widgets/​godam-video.js Adds Elementor title warnings.
assets/​src/​blocks/​godam-player/​utils/​index.test.js Tests filename detection.
assets/​src/​blocks/​godam-player/​utils/​index.js Adds filename detection helpers.
assets/​src/​blocks/​godam-player/​edit.js Prefers HLS SEO URLs.
assets/​src/​blocks/​godam-player/​components/​VideoSEOModal.js Adds warnings and exclusion control.
assets/​src/​blocks/​godam-player/​components/​video-seo-modal.scss Styles filename warnings.
assets/​src/​blocks/​godam-player/​block.json Adds the exclusion attribute.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread inc/classes/class-seo.php Outdated
Comment thread inc/classes/class-seo.php Outdated
Comment thread inc/classes/class-video-sitemap.php Outdated
Comment thread inc/classes/class-video-sitemap.php Outdated
@KMchaudhary

Copy link
Copy Markdown
Collaborator

Code review — VideoObject schema + video sitemap

Overall this is a solid, well-tested change (the tests/php coverage and the schema↔sitemap agreement test are especially nice). A few things worth a look before merge, roughly in priority order:

1. isFilenameLikeTitle flags legitimate hyphenated/underscored titles — assets/src/blocks/godam-player/utils/index.js:163

if ( ! hasSpaces && /[-_]/.test( trimmed ) ) {
    return true;
}

Any single word with a hyphen or underscore and no space is treated as a file name. Real titles trip this: "Spider-Man", "COVID-19", "e-commerce", "Add-ons", "Q3_review". All three authoring UIs (block Notice, Elementor help text, WPBakery warning) will then wrongly tell the user their title "looks like a file name." Consider requiring a stronger signal (e.g. a known media extension, or the title actually matching the file name) rather than "any hyphen/underscore in a single word."

2. Sitemap cache is fully invalidated on unrelated edits — inc/classes/class-video-sitemap.php:85

add_action( 'edit_attachment', array( $this, 'invalidate_cache' ) );
...
add_action( 'update_option_rtgodam-settings', array( $this, 'invalidate_cache' ) );

invalidate_cache() bumps a global version, discarding every cached sitemap page. It fires on any attachment edit (e.g. changing alt text on an unrelated image) and on any GoDAM settings save (not just the SEO toggle). Rebuilding is expensive — a full WP_Query over all public pages plus a per-video build_video_object_schema() for each page. On sites that touch media regularly the transient is effectively never warm. Worth narrowing: only invalidate on attachment edits when the attachment is a video / is referenced by a cached page, and only on the settings save when the SEO/sitemap portion actually changed.

3. Index can advertise sitemap pages that render empty — inc/classes/class-video-sitemap.php:604

get_total_count() / get_max_pages() count posts that merely have VIDEO_SEO_SCHEMA_META_KEY, but get_url_entries() further drops noindexed pages, per-video-excluded videos, and videos that fail is_valid_video(). So get_index_items() and the core/Yoast/Rank Math/AIOSEO index entries can list pages (and inflated counts) whose <urlset> comes back empty — which Search Console reports as empty sitemaps. (No videos are lost across page boundaries, so this is cosmetic/SEO-wart rather than data loss.)

4. Stale-response race in the WPBakery filename warning — assets/src/js/wpbakery/wpbakery-video-selector-param.js:264

updateFilenameTitleWarning() resolves fetchAttachmentData() and then mutates the warning node with no request sequencing or abort. Switching videos (or editing the headline/override) twice quickly can let the earlier request resolve last and show/hide the warning based on the stale attachment. A request token or abort-on-new-call would fix it.

5. transcript_to_text can drop valid spoken lines — inc/classes/class-seo.php:648

if ( preg_match( '/^(WEBVTT|NOTE|STYLE|REGION)\b/', $lines[0] ) ) { continue; }

A whole cue block is dropped when its first text line starts with NOTE/REGION/etc., and separately any cue line that is purely digits is skipped. Legitimate caption text like "Note this down…", "Region four results", or a standalone "2024" is lost from the fallback description excerpt. Minor (fallback path only), but a tighter match (only treat these as headers outside cue bodies) would avoid it.


🤖 Generated with Claude Code

@subodhr258 subodhr258 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at d7368b1: read the PHP and JS, ran PHPUnit (408 pass), and reproduced the issues below with small scripts against the PR's code. Not run in a browser or against live Yoast, Rank Math or AIOSEO installs.

Requesting changes for one security issue (inline on class-seo.php line 1090): the page-text description fallback lets an Author put </script> into the JSON-LD and run their own script on the page. The rest are smaller.

Earlier feedback

Earlier comment At d7368b1
Filename warning flags "Spider-Man", "COVID-19" Fixed. One new false positive and one common miss, inline
Cache cleared on any attachment edit or any settings save Fixed for those two. Still cleared on almost every post save, inline
Index lists pages that come out empty Fixed. The index now builds every page to count it, inline
WPBakery filename warning race Fixed (ran the file with a slow then a fast request; the late one is ignored)
transcript_to_text() drops NOTE and numeric lines Fixed
Copilot: uploads path containment Fixed. A null-byte fatal sits next to it, inline
Copilot: WebVTT cue IDs Fixed
Copilot: sitemap stale after HLS finishes Fixed (post meta hooks)
Copilot: <video:title> over 100 characters Fixed

The Copilot threads have no replies yet. A "fixed in d7368b1" on each would help whoever looks next.

Claims

Claim How checked Result
contentUrl is HLS, never .mpd, fixed without a re-save Code True for block pages. Old Elementor and WPBakery pages lose contentUrl and get no embedUrl until re-saved, inline
Description is never "": own, then transcript, then page text Code + script True. The page-text fallback has the security issue and leaks password-protected text, inline
Sitemap XML is valid for Google Generated output checked with xmllint against Google's sitemap-video.xsd Valid
GoDAM serves the XML before Yoast, Rank Math, AIOSEO and core Read their source True: parse_request at priority 1 runs first
Index hooks for Yoast, Rank Math, AIOSEO and core Read their source Hook names and return shapes are right
Off by default, 404 while off, the setting needs manage_options Code True
godam_video_sitemap_enabled forces it on or off in code Code Only from plugins loaded before GoDAM, inline
Listed in /wp-sitemap.xml when no supported SEO plugin is active Code + core Not when core sitemaps are off, inline
PHP 7.4 Grep for PHP 8-only syntax; min WP is 6.5, so str_* are polyfilled Clean

CI: "Check the committed POT matches a fresh build" fails because the new strings aren't in languages/godam.pot. Run npm run build:prod && composer pot and commit the result.

godam-for-woo: it calls only Seo::get_instance(), get_seo_from_attachment() and the constants, and those signatures are unchanged, so nothing breaks. One thing to know: Woo runs save_seo_data_as_postmeta() right away on woocommerce_product_set_stock_status (a sale can trigger it), and the save path now fetches transcripts. Failed fetches aren't cached, so a dead transcript URL adds up to 3 s per video on every stock status change. Caching a failure for an hour would stop that.

Not tested: a real browser or block editor, live SEO plugin installs, multisite, Search Console.

Comment thread inc/classes/class-seo.php Outdated
Comment thread inc/classes/class-seo.php
Comment thread inc/classes/class-seo.php
Comment thread inc/classes/class-seo.php Outdated
Comment thread inc/classes/class-video-sitemap.php Outdated
Comment thread inc/classes/class-video-sitemap.php
Comment thread inc/classes/class-video-sitemap.php
Comment thread assets/src/blocks/godam-player/utils/index.js Outdated
Comment thread pages/godam/components/tabs/SeoSettings/SeoSettings.jsx Outdated
Comment thread assets/src/js/wpbakery/wpbakery-video-selector-param.js Outdated

@subodhr258 subodhr258 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 4aab4f3: read every change since d7368b1, ran PHPUnit (418 pass), ran the title check on sample titles, and test-merged the branch into today's develop. Not run in a browser.

The security issue is fixed. Both JSON-LD outputs now escape <, > and &, and the page text and transcript are stripped again after decoding. The new test covers the &lt;/script&gt; case. My change request was for that issue; what's left below is smaller.

Earlier comments

Comment At 4aab4f3
An Author can close the JSON-LD tag (security) Fixed
Password-protected page text goes into the schema Fixed, with a test
Old Elementor and WPBakery pages lose contentUrl and embedUrl Fixed: those pages are re-saved once in the background. One scale note inline
%00 in a transcript URL breaks the save on PHP 8 Fixed
Sitemap cache cleared on almost every save Fixed: only when the post had video SEO before
godam_video_sitemap_enabled ignored from a theme Fixed: decided at after_setup_theme, with a test
Cold build runs per-post queries Fixed: caches primed in one go
Title check flags "Paris-2024", misses screen-recording titles Both fixed, but the new comparison flags clean titles too, inline
Settings text wrong when core sitemaps are off Fixed, plus a Sitemap: line in robots.txt
vc.reload never fires Fixed (removed)
Woo stock changes wait on a dead transcript URL Fixed: a failed fetch is remembered for an hour

Still needed before merge: the translation file (POT). "Check the committed POT matches a fresh build" still fails. The last commit put back develop's POT from 16 Oct, so none of this PR's new strings are in it, and develop has changed the POT again since (#2207, the admin menu fix). That makes the POT the one file GitHub shows as conflicting. This order clears both:

  1. Merge develop into this branch.
  2. Run npm run build:prod && composer pot from a folder named godam. The POT header takes the plugin slug from the folder name, so running it in a worktree with another name fails the check.
  3. Commit the result.

@since: the new code now says @since 2.4.1 in about 30 places (it said 2.4.0 before). PRs to develop use @since n.e.x.t, which the release step replaces with the version it actually ships in.

PR description: a few lines no longer match the code: "Failed fetches aren't cached" (now cached for an hour), "corrected without a re-save" (true for blocks; Elementor and WPBakery pages get a background re-save), the filename rule ("flagged when it has hyphens or underscores and no spaces"), and the new robots.txt line isn't mentioned.

Not tested: a browser, live SEO plugins, WP-Cron running the re-save on a real site.

Comment thread assets/src/blocks/godam-player/utils/index.js Outdated
Comment thread inc/classes/class-seo.php

@subodhr258 subodhr258 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more, found while re-reviewing #2194: on category, tag and author archives the description fallback reads an unrelated post, private ones included (inline on line 1197). It is a privacy issue, so my change request stays open until it is fixed. Everything else is in the review above.

Comment thread inc/classes/class-seo.php

@subodhr258 subodhr258 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 854cb2d. Everything from my reviews is fixed: archive pages no longer read another post (is_singular() plus is_post_publicly_viewable()), the title check leaves clean titles alone, old pages are re-saved in batches, and @since is n.e.x.t. PHPUnit (542) and the JS tests pass.

The red POT check isn't needed for merge. The POT is regenerated at the develop to main sync, so please ignore my earlier note about it.

Output each GoDAM video chapter as a schema.org Clip in the VideoObject's
hasPart, so Google can show "key moments" (jump-to links) for the video
in search results.

- Add Seo::get_key_moment_clips(): reads the chapters from the
  attachment's rtgodam_meta at output time, so editing chapters updates
  the schema without re-saving the page.
- Each Clip has name, startOffset, endOffset and a deep link of the form
  <page>?t=<seconds>#godam-video-<attachment id>, which the GoDAM player
  already uses to seek on page load.
- A clip ends at the chapter's own endTime, else the next chapter's
  start, else the video duration (clamped to the duration when known).
- Skip chapters with no title, an unreadable start, a start at or past
  the end of the video, or zero length; drop duplicate start times;
  strip HTML from chapter names.
- Parse chapter times as plain seconds, mm:ss or hh:mm:ss(.ms), falling
  back to originalTime when startTime is missing.
- Build clip URLs without add_query_arg(): use & on plain permalinks and
  drop any existing fragment from the page URL.
- Only add hasPart when the video has an attachment and a host page
  (render-time output on archives/404 has nothing to deep-link to).
- Add filters godam_video_seo_key_moments_enabled and
  godam_video_seo_key_moments.
- Move ISO 8601 duration parsing to Seo::iso8601_to_seconds();
  Video_Sitemap::iso8601_duration_to_seconds() now delegates to it.
- Add VideoSeoKeyMomentsTest (27 tests) and a get_permalink() test stub.
@rudrakshi-gupta
rudrakshi-gupta merged commit 858dc89 into develop Oct 20, 2026
6 of 7 checks passed
@rudrakshi-gupta
rudrakshi-gupta deleted the feature/1622-video-seo-sitemap branch October 20, 2026 08:20
subodhr258 added a commit that referenced this pull request Oct 22, 2026
* Stop the admin menu jumping when a GoDAM page loads (#2207)

* Fix: stop the admin menu jumping when a GoDAM page loads

pages/style.css restores Tailwind's border reset for every element on the
page. It is injected by page-css.min.js after the page has painted, and
core's a { transition: border .05s } then animates each admin menu link's
border from 3px to 0: every item grows from 34px to 40px and the whole
menu jumps by about 186px, then snaps back. Keep the reset off
#adminmenumain and #wpadminbar.

* Fix: open the Media Editor with the admin menu already folded

The editor folds the menu from React after the page has painted, so the
menu showed at full width and then collapsed. Add the folded body class
on the server when the editor opens a file.

* Keep the reset off the menu and admin bar containers too; test the editor fold

- pages/style.css also excludes #adminmenumain and #wpadminbar themselves,
  not only their descendants.
- MediaEditorMenuFoldTest: the Media Editor with an id gets 'folded';
  its file picker, other GoDAM pages, the Media Library and no screen
  do not.

* Regenerate the POT: 12 references into class-pages.php moved down

Line numbers only; no strings added or changed. Built like CI: Node
22.14.0, npm ci, npm run build:prod, composer pot.

* feat(seo): fix VideoObject schema and add video sitemap (#2186)

* feat(seo): fix VideoObject schema and add video sitemap (#1622 v1)

Make GoDAM's video schema indexable by Google and add a video sitemap
that plugs into Yoast SEO, Rank Math, All in One SEO or the WordPress
core sitemap. No new public URLs are created.

- Schema contentUrl now uses the HLS (.m3u8) stream, falling back to the
  original file; the DASH .mpd is never output. Resolved at output time,
  so pages cached before this change are fixed without a re-save.
- Add embedUrl pointing at GoDAM's existing video-embed player page.
- Never output an empty description: fall back to a 160-char transcript
  excerpt (fetched on save only, cached in _godam_seo_transcript_excerpt),
  then the page excerpt, otherwise omit the property.
- Output isFamilyFriendly as a real JSON boolean (string values such as
  "false" are parsed correctly) and omit an empty uploadDate.
- Add Video_Sitemap: /godam-video-sitemap.xml with one <url> per
  published page and one <video:video> per video, built from the cached
  video SEO post meta via the shared schema builder so both always agree.
- Exclude drafts, private and password-protected posts, pages noindexed
  in Yoast / Rank Math / AIOSEO, and videos the editor excludes.
- List the sitemap in the Yoast, Rank Math and AIOSEO indexes, or via a
  core sitemap provider when none is active; paginate at 1000 URLs,
  cache per site with version-based invalidation.
- Add GoDAM > Settings > SEO with an "Enable video sitemap" toggle (off
  by default) and a per-video "Exclude from video sitemap" toggle in
  Gutenberg, Elementor and WPBakery.
- Warn editors in all three editors when a video title looks like a
  raw file name.
- Prefer the HLS URL when prefilling SEO contentUrl in the editors.
- Add new filters: godam_video_seo_content_url, godam_video_seo_embed_url,
  godam_video_seo_description, godam_video_sitemap_enabled,
  godam_video_sitemap_entries_per_page, godam_video_sitemap_post_types,
  godam_video_sitemap_exclude_post, godam_video_sitemap_entry.
- Add PHPUnit coverage (VideoSeoSchemaTest, VideoSitemapTest) and Jest
  tests for the filename-title check.

* Resolve feedback

* Resolve feedbacks and regenrate POT file

* Revert the POT file

* Resolve Privacy Issue

* feat(seo): add key moments to video schema from chapters (#1597) (#2194)

Output each GoDAM video chapter as a schema.org Clip in the VideoObject's
hasPart, so Google can show "key moments" (jump-to links) for the video
in search results.

- Add Seo::get_key_moment_clips(): reads the chapters from the
  attachment's rtgodam_meta at output time, so editing chapters updates
  the schema without re-saving the page.
- Each Clip has name, startOffset, endOffset and a deep link of the form
  <page>?t=<seconds>#godam-video-<attachment id>, which the GoDAM player
  already uses to seek on page load.
- A clip ends at the chapter's own endTime, else the next chapter's
  start, else the video duration (clamped to the duration when known).
- Skip chapters with no title, an unreadable start, a start at or past
  the end of the video, or zero length; drop duplicate start times;
  strip HTML from chapter names.
- Parse chapter times as plain seconds, mm:ss or hh:mm:ss(.ms), falling
  back to originalTime when startTime is missing.
- Build clip URLs without add_query_arg(): use & on plain permalinks and
  drop any existing fragment from the page URL.
- Only add hasPart when the video has an attachment and a host page
  (render-time output on archives/404 has nothing to deep-link to).
- Add filters godam_video_seo_key_moments_enabled and
  godam_video_seo_key_moments.
- Move ISO 8601 duration parsing to Seo::iso8601_to_seconds();
  Video_Sitemap::iso8601_duration_to_seconds() now delegates to it.
- Add VideoSeoKeyMomentsTest (27 tests) and a get_permalink() test stub.

* Match WPBakery SEO Settings to Gutenberg Model (#2209)

* ci: run the POT check only on PRs into main (#2188)

* ci: run the POT check only on the develop → main merge

Feature PRs into develop no longer have to regenerate languages/godam.pot.
The release PR from develop into main still fails if the committed POT is
stale. PRs into main from any other branch skip the job.

* ci: run the POT check on every PR into main

Patch releases reach main from fix and hotfix branches, not only from
develop, so they need the check too. The branches: [main] trigger
already keeps it off feature PRs into develop.

* Pass only a numeric form ID to the form layer preview (#2223)

Cast the layer id to an integer in the preview template and in the Ninja Forms
and MetForm callbacks, and add unit tests for it.

* Keep the Gravity Forms recorder working after an AJAX page change (#2217)

* Keep the Gravity Forms recorder working after an AJAX page change

Gravity Forms replaces a page's markup on every AJAX page change or failed
submit. The recorder kept pointing at the replaced container, file input and
button, and Uppy had bound its dashboard trigger once to the old button, so
Start Recording opened nothing on the new page.

On every gform_post_render, an uploader whose markup left the page now finds
the new container by its id, reads the input and button from it, binds the
new button once and shows the recording it holds again.

* Regenerate the translation template for the recorder's moved lines

* Stop sending the signed-in viewer's email with video analytics (#2213)

* Stop sending the signed-in viewer's email with video analytics

The page params that GoDAM prints on every front-end page no longer carry
the current user's email address (`emailId`), and the analytics request body
no longer has an `email` field. The WordPress user id is still sent.

Tests: the request-body helper and the page params are both checked for the
absence of the email, including a page cached by an older version.

* Remove the viewer's email from the analytics data listed in the readme

The analytics.godam.io entry under External services no longer lists Email as data sent.

* Keep the signed-in viewer's email out of the player's page data

The player script's page data (godamData.currentLoggedInUserData) carried the current user's email address, which nothing in the browser reads. It now holds the display name and whether someone is signed in. The comments routes still read the email on the server.

* Keep the fixture lines untouched in the JS test

The explanation moves into the test, so the fixture lines this branch leaves alone cannot conflict with the sibling change.

* List the page's user fields instead of removing one

rtgodam_get_current_user_data_for_page() now returns only the display name and the sign-in type, so a field added later to the server-side user data is not printed into the page by accident.

* Regenerate the POT for the new line numbers

The new helper in custom-functions.php moves five strings further down the file, so their source references change. Nothing else in the file differs from develop.

* Clean up video share popup: remove GoDAM Central/Embed links, use Page link, show share button for all videos (#2211)

* Clean up video share popup links and share button gating

- Remove the redundant GoDAM Central and Embed copy fields from the share
  modal footer, keeping only the Page Link field.
- Point the social share links at the WordPress page link instead of the
  GoDAM Central URL, which is private by default and shows a login prompt.
- Show the share button for all videos when sharing is enabled in the
  block settings, instead of only transcoded videos.

Closes #2210

* Address review: keep share anchor and gate button on an ID

- Encode the share link with encodeURIComponent so the #godam-video-{id}
  anchor and any query param travel with the social share URL instead of
  splitting off (encodeURI left #, ? and & intact).
- Hide the share button for players with neither a WordPress attachment ID
  nor a transcoding job ID ("Insert from URL" embeds), since their share
  link could not target the video.
- Remove stale README references to the removed GoDAM Central/Embed links
  and correct the ID described for share links.

---------

Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>

* Save converted MP3 audio as audio/mpeg, and fix files already saved as audio/mp3 (#2208)

* Fix: store transcoded MP3 audio as audio/mpeg, not audio/mp3

WordPress registers MP3 as audio/mpeg. Attachments saved as audio/mp3
were left out of REST media_type=audio, and REST ignored a
mime_type=audio/mp3 filter. New conversions are saved as audio/mpeg,
and a one-time migration (2.4.1 batch) fixes the ones saved before.

* Test the saved type, satisfy the hook-integrity check, retry a failed SELECT

- TranscodedAudioMimeTypeTest calls add_transcoded_files() and checks
  that converted audio is saved as audio/mpeg (video stays video/mp4).
  With the old audio/mp3 it fails.
- add_transcoded_files() starts $transcoded_files as array(), not false;
  PHP 8.1+ reported every conversion as a deprecated false-to-array use.
- The migration's two direct queries carry godam-coverage-ignore: the
  runner already runs it once per site.
- A failed SELECT now returns false so the runner retries, instead of
  marking the site done with nothing changed.
- The runner test checks batch order instead of which batch is last.

* Run the MP3 type migration in the 2.5.0 batch

2.4.1 ships without this change but still stores 2.4.1 as each site's
migration version, and the runner skips batches at or below the stored
version. Keyed 2.4.1, the migration would never run on sites that update
through 2.4.1; keyed 2.5.0 it runs for both 2.4.0 and 2.4.1 sites.

* Release prep v2.5.0: version bump, changelog and POT refresh (#2231)

- Version 2.5.0 in godam.php, readme.txt, README.md, package.json and package-lock.json.
- v2.5.0 changelog in readme.txt and CHANGELOG.md; v2.3.2 removed from readme.txt to keep the last three.
- Every @SInCE n.e.x.t replaced with 2.5.0 (51 tags in 6 files).
- languages/godam.pot regenerated from a production build.

* Fix the two new Plugin Check warnings in the Video SEO sitemap code (#2232)

- class-video-sitemap.php: the phpcs:ignore named slow_query_meta_key, which is not a sniff, so it ignored nothing. Use slow_db_query_meta_key.
- class-video-sitemap-core-provider.php: ignore NonPrefixedHooknameFound on WordPress core's own wp_sitemaps_index_entry filter.

* Keep page-builder shortcode text out of the video sitemap description (#2235)

* Keep page-builder shortcode text out of the video sitemap description

The sitemap is served at parse_request priority 1, before WPBakery and Divi
register their shortcodes, so core's strip_shortcodes() left tags such as
[vc_row] in the fallback description taken from the page text. Add the tag
names found in the text to strip_shortcodes()'s own list so unregistered
shortcodes are removed the same way registered ones are, which gives the same
description in the sitemap and on the front end.

The strip_shortcodes() test stub removed every [tag], which hid the bug. It now
follows core: only registered shortcodes are removed.

* Only strip builder-style unregistered shortcode names from the fallback description

Review of the first version found that it also removed ordinary bracketed text
([sic], [1], [b]bold[/b]) from the front-end description, that one very long
bracketed word made core's pattern too large and the description empty, that a
tag another filter had taken off the strip list was re-added, and that the
filter stayed registered if stripping threw.

Only names that look like a builder tag are added now (a letter first, an
underscore or hyphen in the name, 64 characters at most), tags that are
registered are left to core, and the filter is removed in a finally block.
New tests pin the cap, the length limit, the filter removal and the ordinary
bracketed text.

* Set the 2.5.0 changelog date to October 22, 2026 (#2236)

2.5.0 releases on 22 Oct (release ticket rtCamp/godam-plugin-wp#60), but readme.txt and CHANGELOG.md dated it October 21.

---------

Co-authored-by: Kuldip Chaudhary <64731232+KMchaudhary@users.noreply.github.com>
Co-authored-by: Rudrakshi Gupta <109859631+rudrakshi-gupta@users.noreply.github.com>
Co-authored-by: Vishal Kumar <vishnshiv3@gmail.com>
Co-authored-by: KMchaudhary <kuldipkumar.chaudhary@rtcamp.com>
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.

4 participants