Skip to content

Render the GoDAM player from attributes instead of a built shortcode string - #2243

Merged
subodhr258 merged 2 commits into
mainfrom
fix/render-player-from-attributes
Oct 26, 2026
Merged

subodhr258 merged 2 commits into
mainfrom
fix/render-player-from-attributes

Conversation

@subodhr258

Copy link
Copy Markdown
Collaborator

What changes

Several places built a [godam_video ...] shortcode string from request or form-entry values and passed it to do_shortcode(). They now pass an attributes array to the registered [godam_video] callback through a new helper, rtgodam_render_shortcode_from_atts( $tag, $atts ) in inc/helpers/custom-functions.php. A value can only ever be an attribute value, never shortcode syntax.

Where Change
Video embed page, godam_embed_page_content() godam_context and block_source go through sanitize_key(); player rendered from an array. The preview page uses the helper too
godam/v1/video-shortcode route godam_context uses sanitize_key (route arg and handler); player rendered from an array, so the __rtgob__ bracket placeholders are no longer needed on the default path
Recorder entry views: Fluent Forms, WPForms (view and edit), Everest Forms, SureForms, Gravity Forms stored recording and transcoded URLs passed as attribute values (esc_url_raw())
GoDAM Gallery Elementor widget widget settings passed as an array to [godam_video_gallery]

Filters on the route:

  • New rtgodam_shortcode_atts( $atts, $id ) filters the attributes array.
  • rtgodam_shortcode_output( $shortcode, $id ) still works when a site uses it: the string is built as before and the filter runs through apply_filters_deprecated(). Every value in that string is now an ID, a slug, show/hide or JSON.

Rendered HTML for normal values is unchanged.

Testing

Throwaway WordPress (Playground, WordPress 7.1.3, PHP 8.3) with the 2.5.0 release plus these files and Fluent Forms 6.2.16. A test shortcode registered on the site records whether it runs.

Check Result
Embed page with normal godam_context and block_source player renders, data-block-source kept
Embed page with extra shortcode text in godam_context or block_source test shortcode not run, player still renders
video-shortcode route, normal call success, player HTML
video-shortcode route with extra shortcode text in godam_context test shortcode not run, player still returned
Fluent Forms entry (admin, /fluentform/v1/submissions/<id>), normal recording player with the recording URL
Fluent Forms entry whose stored value has extra shortcode text test shortcode not run, player present
HTML for normal input (embed page, route, Fluent entry) identical to 2.5.0 apart from random player IDs, nonces and timestamps
  • PHPUnit: 11 new test files, one per entry point plus the helper and the deprecated filter; each checks a normal value reaches the player with the same attributes and that appended shortcode text does not run. Full suite OK (665 tests).
  • phpcs clean on every changed file; bin/hook-check balance and coverage checks pass.
  • WPForms, Everest Forms, SureForms, Gravity Forms and the Elementor widget are covered by unit tests only, not an end-to-end run.

Worth knowing

  • godam_context and block_source are reduced to slug characters (lowercase letters, digits, - and _). Every context GoDAM and GoDAM for WooCommerce send already is one.
  • An unregistered [godam_video] now renders nothing instead of the literal shortcode text.

QA:

  1. Open a video's embed link and a gallery lightbox video: both play.
  2. Submit a recording through each form plugin with the GoDAM recorder field and open the entry in wp-admin: the recording shows and plays.
  3. Open a Woo product gallery or Reel Pop video (they send a godam_context): plays as before.

Issue: rtCamp/godam-plugin-wp#71

@subodhr258 subodhr258 self-assigned this Oct 22, 2026
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
13 0 13

⚠️ Warnings (13)

📁 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.
📁 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/26/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/26/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

…string

The video embed page, the video-shortcode REST route, the GoDAM Gallery
Elementor widget, the preview page and the entry screens of the Fluent Forms,
WPForms, Everest Forms, SureForms and Gravity Forms recorder fields now hand
their values to the player as an attributes array. A new helper,
rtgodam_render_shortcode_from_atts(), calls the registered shortcode callback
directly, so a value is only ever an attribute value and is never parsed as
shortcode syntax.

- The embed page and the route reduce godam_context and block_source to slug
  characters with sanitize_key(); the route argument uses sanitize_key too.
- Stored recording and transcoded URLs are passed through esc_url_raw().
- The route gets a rtgodam_shortcode_atts filter for the attributes array.
  The rtgodam_shortcode_output filter, which receives the shortcode string, is
  deprecated but still applied when a site uses it; every value in that string
  is now an ID, a slug, 'show'/'hide' or JSON.

The rendered player HTML for normal values is unchanged.

Adds unit tests for the helper, the embed and preview pages, the route, the
Elementor gallery widget and each form plugin's entry view. The tests register
a harmless shortcode and check that extra shortcode text in a value does not
run it, and that a normal value still reaches the player.
@subodhr258
subodhr258 changed the base branch from develop to main October 23, 2026 06:21
@subodhr258
subodhr258 force-pushed the fix/render-player-from-attributes branch from b7cdaae to c79ab5b Compare October 23, 2026 06:21
@subodhr258

Copy link
Copy Markdown
Collaborator Author

Tested together with the other five 2.5.1 PRs on a hosted test site connected to production GoDAM Central. Full table and all recordings: https://github.com/rtCamp/godam-plugin-wp/issues/68 (recordings are in the rtCamp Test media library; sign in to watch).

Tested commit b7cdaaeaa; the current head c79ab5b41 has the same code.

  • A published video renders from the video-shortcode route for a logged-out visitor: step 4 of T9. The GoDAM player also renders on a published post and page in T3 and T5.
  • Not covered: the Elementor gallery widget and the form-recorder entry screens.

@elifvish elifvish 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.

No issues found. One thing worth knowing for third parties: rtgodam_render_shortcode_from_atts() calls the callback directly, so pre_do_shortcode_tag / do_shortcode_tag no longer fire on these paths. Nothing in rtCamp hooks them for godam_video.

Verification

  • Head tested: c79ab5b41c. PHPUnit on this PR's head: 665/665 OK.
  • All seven 2.5.1 PRs merged onto main (no conflicts): PHPUnit 915/915, Jest 690/690, build:prod OK.
  • Playwright API checks against that build on a local WordPress 7.1.3 site: 4/4 for this PR.
  • Not covered: Form entry views (Fluent, WPForms, Everest, SureForms, Gravity) with a real recording: no recorder submissions on test-this; covered by the PR’s unit tests; Elementor gallery widget render.

Screenshots and a recording of the local run are available on request.

@KMchaudhary

Copy link
Copy Markdown
Collaborator

Review — Approvable ✅

Reviewed at high effort. This is a clean, well-scoped fix for the shortcode-string injection class.

Verified:

  • rtgodam_render_shortcode_from_atts() invokes the shortcode callback directly with an atts array, so no attribute value is ever re-parsed as shortcode syntax — the right fix.
  • godam_video::render() takes only $atts; the extra call_user_func args are harmless and the $content empty-string-vs-null distinction doesn't apply.
  • sources JSON with real [/] round-trips correctly through render()'s placeholder str_replace on the new direct path; the deprecated do_shortcode path still uses placeholders. Both correct.
  • Elementor gallery atts keys are all lowercase, so bypassing the parser's name-lowercasing changes nothing.
  • Form entry points correctly use esc_url_raw() for values passed as data; HTML-output audio branches keep esc_url().
  • Legacy rtgodam_shortcode_output preserved via apply_filters_deprecated(); new rtgodam_shortcode_atts filter added. 11 new tests, suite green.

Non-blocking notes (follow-up, not this PR):

  • Form-embed shortcodes still build strings from values ([everest_form … title='…'], [wpforms …], [gravityform …], [metform …], [sureforms …], [forminator_form …], [contact-form-7 …], plus [poll id='…'] / do_shortcode($godam_cta_html) in the player template). Out of this PR's player scope and not a regression here — consider a follow-up ticket if any of those values can be untrusted.
  • A site attaching both the new atts filter and the legacy output filter would drop atts added only via the new filter on the legacy string path. Edge case, fine for a hotfix.

No blocking issues. 👍

@subodhr258
subodhr258 merged commit e5026d1 into main Oct 26, 2026
9 of 10 checks passed
@subodhr258
subodhr258 deleted the fix/render-player-from-attributes branch October 26, 2026 07:15
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.

3 participants