Stop adding “noreferrer” to external links.#26968
Conversation
|
Looks like this dropped off my radar. I will try to revisit soon and land this. As far as I can tell, the only outstanding item (other than resolving merge conflicts) was adding a deprecation for the file block. Also, I will review the codebase to make sure more |
The deprecated save functions must match old saved content in the database (which has noreferrer noopener). Only the current save function should output noopener alone.
|
@Krinkle - thanks for reviewing. I have resolved merge conflicts, this should be ready to merge once CI is green. |
jsnajdr
left a comment
There was a problem hiding this comment.
Looks good 👍 Given that we now have the global Referrer-Policy and that modern browsers send only origin by default, the original reasons why referrers were dangerous (leaking URL details) no longer hold, they've been solved at web platform level.
Similarly, modern browsers set noopener automatically for target="blank", so the second ubiquitous rel value can be removed, too. That will also completely remove a lot of the NEW_TAB_REL code.
| rel={ | ||
| textLinkTarget ? 'noreferrer noopener' : undefined | ||
| } | ||
| rel={ textLinkTarget ? 'noopener' : undefined } |
There was a problem hiding this comment.
This change may break blocks, so I think we'll need to add a new migration. Try the following steps:
- Check out the trunk branch
- Insert a File block
- Attach a PDF file and enable "Open in new tab"
- Save the post
- Check out the
remove/noreferrerbranch - Reload your browser
- The block is broken
Perhaps we can exclude the File block in this PR. To avoid adding multiple block migrations, it might be better to submit a separate PR specifically for the File block and then remove noreferer, noopener all at once.
There was a problem hiding this comment.
We should do a migration. Then it makes sense to remove both noreferrer and noopener in one PR.
We did a similar migration in #43050, for a similarly "banal" change--changing the value of an aria-label attribute. The change doesn't need to be big and serious to warrant a migration script.
There was a problem hiding this comment.
It would be great to land #73116, so we don't have to introduce deprecations for every minor change.
There was a problem hiding this comment.
Perhaps we can exclude the File block in this PR.
Good idea @t-hamano - I am going to exclude the File block for now, and open a separate PR just for it.
We should do a migration.
I will include a migration in the new PR.
It would be great to land #73116, so we don't have to introduce deprecations for every minor change.
@Mamaduka so after that merges, we wouldn't need a migration at all?
There was a problem hiding this comment.
Follow up PR to update the File block, including migration: #77345
The File block's save function deterministically generates the rel attribute. Changing its output invalidates existing saved blocks on reload. Defer to a follow-up PR that removes both noreferrer and noopener together with a proper deprecation to handle migration cleanly.
# Conflicts: # packages/blocks/src/api/raw-handling/phrasing-content-reducer.ts # packages/components/CHANGELOG.md
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.
Resolve CHANGELOG conflict: preserve the ExternalLink breaking-change note in Unreleased and keep the 32.6.0 release section from trunk.
Description
Fixes #26914.
Removes the
noreferrervalue from therelattribute on external links generated by the editor, admin UI, and block output.noopeneris retained where needed to prevent reverse-tabnabbing ontarget="_blank"links.Scope
The change touches two groups of links:
ExternalLinkcomponent, More Menus ineditor/edit-widgets/customize-widgets,media-categories"view more" links, editor-only links insite-logoandcategories): wp-admin sendsReferrer-Policy: strict-origin-when-cross-originviawp_admin_headers— which is also the modern browser default — so only the origin is sent, never full URLs or query strings. Removingnoreferrerhere does not leak sensitive admin paths.latest-posts,button,imageblock output; the link format informat-library/src/link/utils.js; the phrasing-content paste reducer; and the associated test fixtures): sharing the referrer is the expected web-platform default, and the site's own Referrer-Policy governs what is sent.File block — excluded from this PR
Per @t-hamano's review, changes to the File block have been deferred to a separate follow-up PR #77345. The File block's
save()function deterministically generates therelattribute, so changing its output invalidates existing saved File blocks on reload. The follow-up will remove bothnoreferrerANDnoopenerin one pass, with a proper new deprecation to migrate existing content cleanly.See discussion: #26914 (comment)
How has this been tested?
npm run test:unit)reloutputTypes of changes
Bug fix / behavior change — removes
noreferrerfrom editor-generated external links. One breaking change for@wordpress/componentsconsumers:ExternalLinkno longer addsnoreferrerby default (documented in the components CHANGELOG).Checklist: