Skip to content

VideoPress: prevent false dirty state in Site Editor for legacy blocks#50256

Open
shsajalchowdhury wants to merge 2 commits into
Automattic:trunkfrom
shsajalchowdhury:fix/videopress-legacy-block-dirty-state
Open

VideoPress: prevent false dirty state in Site Editor for legacy blocks#50256
shsajalchowdhury wants to merge 2 commits into
Automattic:trunkfrom
shsajalchowdhury:fix/videopress-legacy-block-dirty-state

Conversation

@shsajalchowdhury

@shsajalchowdhury shsajalchowdhury commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What

Fixes #48633

Why

When the Site Editor loads a template containing legacy wp:video blocks with VideoPress attributes (guid, videoPressClassNames), the editor persistently shows "Review 1 change" on every reload — even after saving. The dirty state is caused entirely client-side with no actual content change.

Root Cause

In componentDidUpdate, the VideoPress edit component unconditionally calls setAttributes( { videoPressClassNames: sandboxClassnames } ) whenever a preview is available. Even when the computed value is byte-for-byte identical to the existing attribute, the setAttributes dispatch marks the block (and by extension the template entity) as dirty.

How

Added a strict equality check: only call setAttributes when sandboxClassnames differs from the current attributes.videoPressClassNames value. This prevents the unnecessary dispatch that flags the entity as dirty.

Testing Instructions

  1. Open the Site Editor on a site with Jetpack/VideoPress active
  2. Edit a template via Code Editor and insert a legacy VideoPress block:
<!-- wp:video {"id":1,"guid":"239t8ryY","videoPressClassNames":"wp-block-embed is-type-video is-provider-videopress"} -->
<figure class="wp-block-video wp-block-embed is-type-video is-provider-videopress"><div class="wp-block-embed__wrapper">
https://videopress.com/v/239t8ryY?resizeToParent=true&amp;cover=true&amp;preloadContent=metadata&amp;useAverageColor=true
</div></figure>
<!-- /wp:video -->
  1. Save the template
  2. Reload the Site Editor
  3. Before: "Review 1 change" appears immediately
  4. After: No unsaved changes indicator

Changelog

VideoPress: prevent persistent "Review 1 change" in the Site Editor caused by unnecessary setAttributes calls when the videoPressClassNames value has not changed.

Does this pull request change what data or activity we track or use?

No. This change only adds a strict equality check before calling setAttributes — no data collection or tracking changes.

Testing instructions

  1. Open the Site Editor on a site with Jetpack/VideoPress active
  2. Edit a template via Code Editor and insert a legacy VideoPress block:
    <!-- wp:video {"id":1,"guid":"239t8ryY","videoPressClassNames":"wp-block-embed is-type-video is-provider-videopress"} --><figure class="wp-block-video wp-block-embed is-type-video is-provider-videopress"><div class="wp-block-embed__wrapper">https://videopress.com/v/239t8ryY?resizeToParent=true&cover=true&preloadContent=metadata&useAverageColor=true</div></figure><!-- /wp:video -->
  3. Save the template
  4. Reload the Site Editor
  5. Before: The "Review 1 change" badge appears immediately
  6. After: No unsaved changes indicator is shown

@github-actions github-actions Bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖



Jetpack plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

@github-actions github-actions Bot added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. OSS Citizen This Pull Request was opened by an Open Source contributor. labels Jul 7, 2026
@simison simison requested a review from obenland July 7, 2026 07:09

@jeherve jeherve left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for your contribution.

We'll need to fix the changelog format before we can merge this.

Comment thread projects/plugins/jetpack/changelog/fix-videopress-legacy-block-dirty-state Outdated
@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jul 7, 2026
@obenland

obenland commented Jul 7, 2026

Copy link
Copy Markdown
Member

Thanks @shsajalchowdhury! It looks like this fixes it for the site editor, but the post editor still shows it dirty. I believe the videoPressTracks attribute being set there might be the culprit. Would you want to look at that, too?

The componentDidUpdate lifecycle unconditionally called setAttributes
with videoPressClassNames on every render when a preview was available.
This marked the entity as dirty even when the value was identical,
causing a persistent "Review 1 change" prompt in the Site Editor.

Fix: Only dispatch setAttributes when the computed sandboxClassnames
differs from the current videoPressClassNames attribute value.

Fixes Automattic#48633
@shsajalchowdhury shsajalchowdhury force-pushed the fix/videopress-legacy-block-dirty-state branch from 1c888dd to b53dcaf Compare July 7, 2026 19:04
@shsajalchowdhury

Copy link
Copy Markdown
Contributor Author

Hi @jeherve, thanks for the review! The changelog type was already corrected from fixed to bugfix in a follow-up commit (b53dcaf). I've rebased on the latest trunk and force-pushed to trigger a fresh CI run. The code change itself is minimal — just a guard to only call setAttributes when sandboxClassnames differs from the current videoPressClassNames attribute value, preventing the unnecessary dirty state. Let me know if anything else needs adjusting!

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

Labels

OSS Citizen This Pull Request was opened by an Open Source contributor. [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Status] Needs Review This PR is ready for review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[VideoPress] Legacy wp:video blocks with VideoPress attributes cause persistent "Review 1 change" in Site Editor

3 participants