Skip to content

File block: Remove rel attribute from text link; add migration#77345

Open
adamsilverstein wants to merge 1 commit into
WordPress:trunkfrom
adamsilverstein:file-block/remove-rel
Open

File block: Remove rel attribute from text link; add migration#77345
adamsilverstein wants to merge 1 commit into
WordPress:trunkfrom
adamsilverstein:file-block/remove-rel

Conversation

@adamsilverstein
Copy link
Copy Markdown
Member

Description

Follow-up to #26968 per @t-hamano's review feedback. The File block change was deferred from #26968 because its save() function deterministically generates the rel attribute, so changing the output invalidates existing saved blocks on reload. This PR handles the File block properly with a migration.

Changes

  • save.js: Remove the rel attribute entirely from the text link <a>. Modern browsers apply noopener implicitly for target="_blank" links (Chrome 88+, Firefox 79+, Safari 12.1+), so no meaningful security regression on supported browsers. noreferrer is also dropped per the broader effort in Stop adding “noreferrer” to external links. #26968 / Stop adding rel=noreferrer to links that open in a new tab #26914.
  • deprecated.js: Add a new v4 deprecation at the top of the list, mirroring the current block's attributes and supports exactly but emitting rel="noreferrer noopener". No migrate/isEligible needed — attributes are identical between v4 and the new current block; only the serialized rel differs, which is the precise shape the deprecation system handles.
  • Test fixtures: Update core__file__new-window.* to reflect the new current save (no rel). Add core__file__new-window__deprecated-2.* to exercise the v4 migration path (input has old rel, serialized output has none).

Verification

  • npm run test:unit -- test/integration/full-content — all 372 fixture tests pass, including the new core__file__new-window__deprecated-2.
  • Manual (per reviewer's suggested steps):
    1. Check out trunk, insert a File block, attach a PDF, enable "Open in new tab", save the post — post content contains rel="noreferrer noopener".
    2. Check out this branch, reload — block loads without a validation warning and upgrades cleanly via v4.
    3. Re-save — content now has no rel attribute.

Context

Types of changes

Behavior change — removes rel="noreferrer noopener" from File block saved output. Existing saved blocks migrate via the new v4 deprecation with no content loss.

Checklist

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

The File block's save function previously added rel="noreferrer noopener"
to the text link when opened in a new tab. Modern browsers apply noopener
implicitly for target="_blank" (Chrome 88+, Firefox 79+, Safari 12.1+),
and noreferrer blocks legitimate referrer tracking without meaningful
security benefit on supported browsers.

Add a new v4 deprecation mirroring the current block schema but with the
previous save output so existing saved blocks migrate cleanly on reload.

Fixture added: core__file__new-window__deprecated-2 exercises the v4
migration path (input has rel="noreferrer noopener", output has no rel).

Follow-up to WordPress#26968 per review feedback from @t-hamano.
@github-actions github-actions Bot added the [Package] Block library /packages/block-library label Apr 14, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 14, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@adamsilverstein adamsilverstein added [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement. and removed [Type] Bug An existing feature does not function as intended labels Apr 14, 2026
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.

I don't think this fixture file can correctly check if the v4 migration is working properly. Try to run 'npm run fixtures:regenerate' after deleting the v4 migration. This isValid field should remain true. In other words, this fixture will not break the block even if the v4 migration is absent. To ensure the v4 migration works correctly, I believe we need to validate it using HTML generated from a PDF file.

Additionally, the number 2 might be misunderstood as v2 migration, so a filename like core__file__deprecated-4.html might be better.

@jsnajdr
Copy link
Copy Markdown
Member

jsnajdr commented Apr 15, 2026

The improved block validation levels from #73116 might land soon. How will the new validation features affect this PR? Will the v4 migration be still needed? Without the migration, what will be the validation level of a markup with legacy rel attribute -- ReconstructedBlock? If we land the v4 migration now, can we revert it later when it's not needed?

@adamsilverstein
Copy link
Copy Markdown
Member Author

The improved block validation levels from #73116 might land soon. How will the new validation features affect this PR? Will the v4 migration be still needed? Without the migration, what will be the validation level of a markup with legacy rel attribute -- ReconstructedBlock? If we land the v4 migration now, can we revert it later when it's not needed?

I have the same question and was thinking we should hold off on merging this until clarified.

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

Labels

[Package] Block library /packages/block-library [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants