Make WordPress Core

Opened 21 months ago

Closed 15 months ago

Last modified 8 months ago

#57913 closed enhancement (fixed)

Attachment pages need to have an "on" toggle

Reported by: joostdevalk's profile joostdevalk Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: Media Keywords: add-to-field-guide has-dev-note
Focuses: Cc:

Description

WordPress creates attachment pages by default for every attachment uploaded. On the *vast* majority of sites, these attachment pages are useless. They do however exist, and get crawled, and sometimes even rank in search results, leading to bad results for users and site owners. I want to propose we get rid of them.

Since attachment pages *are* in use on a minority of sites, this needs a bit of a path forward. My proposed solution:

  • We should create a toggle in WordPress core that enables them, and toggle that toggle "off" by default for new sites.
  • On existing sites that toggle should be "on", but setting it to "off" should disable the attachment URLs entirely, redirecting existing attachment URLs to their parent post.

Attachments (3)

57913-toggle-wp-attachment-pages-enabled.zip (1.3 KB) - added by zunaid321 15 months ago.
This plugin adds an admin bar item to toggle the new option. Created by @costdev
57913.2.diff (1.4 KB) - added by joedolson 15 months ago.
Change text links based on attachment page status
57913.3.diff (1.4 KB) - added by ironprogrammer 15 months ago.
Refreshed patch to update option to wp_attachment_pages_enabled.

Download all attachments as: .zip

Change History (79)

#1 @SergeyBiryukov
21 months ago

  • Component changed from General to Media

#2 @jonoaldersonwp
21 months ago

Seconded. This behaviour is actively harmful to the vast majority of websites. We shouldn't rely on users to have to know this, to have to seek out a solution to a problem they don't (and shouldn't) need to understand, and to pester theme developers to support a template type that shouldn't exist in the first place. That's not a sensible solution for 99% of websites / site owners.

#3 follow-up: @johnbillion
21 months ago

I've built websites for photographers in the past using mostly just attachment templates, although I'll admit I'm not sure if I would do the same if I built such as site now.

Is that a consideration? How is a modern photography website or theme structured?

Should such a toggle be on the Permalinks screen? Seems like the ideal place.

#4 @joedolson
21 months ago

It might be reasonable to provide support for this through add_theme_support(), to make a better experience for something like a photography website where that's desirable.

I could see this toggled either in permalinks or in media settings.

@joostdevalk What would you suggest as the redirect destination for unattached media?

#5 @basiliskan
21 months ago

Agreed with that, default behavior needs to be considered based on how things roll nowadays.

#6 follow-up: @audrasjb
21 months ago

Thanks for the ticket, however I believe there is some caveats to address in this proposal:

On existing sites that toggle should be "on", but setting it to "off" should disable the attachment URLs entirely, redirecting existing attachment URLs to their parent post

Attachments do not necessarily have a parent post: unattached medias need to be redirected elsewhere. On the homepage?

Would this change redirect all kind of attachments? Would it force redirection over all the single-attachment-{slug}.php and the various {MIME-type}.php templates (like pdf.php, video.php, text-plain.php, etc)?

#7 follow-up: @davelo
21 months ago

I want to bring in another point besides the whole SEO-discussion, the latest years i have seen more and more talks around wasted energy in tech, and in particular website building. If you give scrapers something to scrape, it will be scraped. We can point to the "scrapers" which won't have any effect, or WordPress can take action itself.

So, if WordPress has it's mouth full of bringing impact to the world and it's environment, this is in my opinion for sure a no-brainer, regardless the SEO-impact which will be very minor compared to the impact on energy consumption.

#8 @rilwis
21 months ago

Regarding redirection, I think it’s better to redirect attachment page to the direct media file URL. Redirecting it to the parent post seems incorrect when users want to search for an image and get a post.

I’ll vote for making it default and maybe add a filter to revert to the existing (current) behavior. Adding an option even confuses users, and I guess there will be a lot of tutorials just to explain “why” this option exists. After a few years, this will be a history.

This ticket was mentioned in PR #4233 on WordPress/wordpress-develop by @aristath.


21 months ago
#9

  • Keywords has-patch added

This PR does the following:

  • Adds an option in Settings > Media to enable/disable the attachment pages
  • Adds an upgrade routine so that existing sites keep the option enabled by default
  • Redirects (301 redirect by default) attachment pages to the attachment-URLs (direct link to the file itself)

Trac ticket: https://core.trac.wordpress.org/ticket/57913

props @carolinan @SergeyBiryukov @aristath

#10 in reply to: ↑ 7 ; follow-up: @manfcarlo
21 months ago

Surely this ticket must be a duplicate? I'd be shocked if this has not been reported in 20 years.

#11 in reply to: ↑ 10 @SergeyBiryukov
21 months ago

Replying to manfcarlo:

Surely this ticket must be a duplicate?

This was indeed suggested in #49010 before, that ticket had a slightly different angle though.

#12 in reply to: ↑ 3 @azaozz
21 months ago

Replying to johnbillion:

I've built websites for photographers in the past using mostly just attachment templates

It might be reasonable to provide support for this through add_theme_support()

+1. Thinking themes should be able to use attachment templates if they want to.

Regarding redirection, I think it’s better to redirect attachment page to the direct media file URL.

Also +1. Thinking this would be the correct behavior. This can also check the "visibility" cap of the current user and output a 404 if the user cannot "see" the attachment post.

Should such a toggle be on the Permalinks screen? Seems like the ideal place.

Not sure if this should be another checkbox in the WP settings. Perhaps seems "too advanced" for most users to make a decision? Imho better to have the functionality in WP but let the theme decide either to enable or disable or to add a checkbox in the theme's settings.

#13 in reply to: ↑ 6 @azaozz
21 months ago

  • Keywords 2nd-opinion added; has-patch removed

Replying to audrasjb:

Would this change redirect all kind of attachments? Would it force redirection over all the single-attachment-{slug}.php and the various {MIME-type}.php templates (like pdf.php, video.php, text-plain.php, etc)?

All very good questions. Seems leaving the decision to the theme would be best. I.e. the theme may decide to include a template for specific type of attachments but not for others, etc. Also seems single-attachment-{slug}.php may need to be an exception? This should probably be reflected when making a patch.

That said, looking at https://github.com/WordPress/wordpress-develop/pull/4233 don't think having a checkbox anywhere in WP is a good idea and also don't think storing an option to (globally) turn visibility of attachment pages on/off is good either. As mentioned above seems this should be an add_theme_support() thing. Will comment on the PR too.

#14 @joostdevalk
21 months ago

I'm 100% on board with an add_theme_support approach and with redirecting to the attachment too.

We could simply detect theme support and then show it, and otherwise redirect to the attachment URL itself? Would mean that people that rely on this to work need to add an add_theme_support option before switching to the version we include this in, but this would be a *huge* win for the web as far as I'm concerned.

So:

  • We'd add an add_theme_support option for this.
  • We default to attachments not being enabled.
  • When they're not enabled we redirect the attachment URLs to the actual attachment itself.
Last edited 21 months ago by joostdevalk (previous) (diff)

#15 @aristath
21 months ago

  • Keywords has-patch added

Thank you everyone for the suggestions and the very constructive discussion!

We updated the PR today and it now follows the summary/outline from the comment above by adding an attachment-pages theme-support, a permissions check and simplifying the previous implementation a lot.

#16 @seedsca
21 months ago

This seems fine for any large site with lots of media files, that does not want them to have their own attachment pages. How about all of the smaller sites that take advantage of this feature? This seems kinda miopic. We already have a way to address this, thanks to plugins. If a site gets too big and/or won't benefit from this feature, then there are options.

Please keep in mind the millions of uses out there.

#17 follow-up: @eatingrules
21 months ago

I'll be glad to see attachment pages go away for sites that don't actually utilize them.

However, it sounds like this proposal has evolved so that it will be turned off for ALL sites in an upcoming release... unless a theme is then updated to include the new filter to turn them back on?

Won't that break all sites that do want attachment pages? You'll then be putting the burden on theme developers _and_ site owners to understand this change, update their code, run the theme update (which may not be a trivial project), and do all that _before_ updating core. ? That seems highly problematic.

(I don't like the idea of the toggle either, but... perhaps this could be more like the old "Links" feature. If it was in use, it stays in use...)

#18 in reply to: ↑ 17 ; follow-ups: @SergeyBiryukov
21 months ago

Replying to eatingrules:

(I don't like the idea of the toggle either, but... perhaps this could be more like the old "Links" feature. If it was in use, it stays in use...)

Yeah, that could be implemented if that's the consensus here:

  • Keep the upgrade routine from the previous iteration of the PR, which would set an option, e.g. wp_attachment_pages_enabled (to follow the link_manager_enabled precedent), so that existing sites don't need to do anything to keep the current behavior.
  • For new sites, set the wp_attachment_pages_enabled to 0 by default.
  • Older sites that want to disable the attachment pages can also set the option to 0.
  • If either of these two values is truthy, keep attachment pages as is:
    • current_theme_supports( 'attachment-pages' )
    • get_option( 'wp_attachment_pages_enabled' )
  • Otherwise, redirect to the attachment URL.

#19 @seedsca
21 months ago

I think these attachment pages could be quite useful for many uses. Considering this is a CMS, and SEO is not the only concern. This takes away a useful function that regular users would not know about, unless they are nerds like us :)

I know I misspelled myopic earlier, but my point stands. We only see what we see. We are talking about samples, even worse, samples from what we care to look at.

I've seen small, indie, sites use this feature well. Same with personal sites. Multisite, and universities come to mind as well... We seem to only be talking about SEO and big sites here. We are much more than that.

If we want to conserve crawl budgets and energy, why not remove these pages from the sitemaps by default? Then have a hook to add them, if wanted...

#20 in reply to: ↑ 18 @eatingrules
21 months ago

I appreciate @seedsca's concerns -- it's a good idea for us to get outside our developer "bubble" whenever we can. How can we learn if there are valuable use cases we're not considering?

In the meantime, @SergeyBiryukov's latest workflow seems like an improvement -- since any sites that are currently using them should keep working okay. (And this is more aligned philosophically with @joostdevalk 's initial proposal, so it's backwards-compatible.)

Yeah, that could be implemented if that's the consensus here:

  • Keep the upgrade routine from the previous iteration of the PR, which would set an option, e.g. wp_attachment_pages_enabled (to follow the link_manager_enabled precedent), so that existing sites don't need to do anything to keep the current behavior.
  • For new sites, set the wp_attachment_pages_enabled to 0 by default.
  • Older sites that want to disable the attachment pages can also set the option to 0.
  • If either of these two values is truthy, keep attachment pages as is:
    • current_theme_supports( 'attachment-pages' )
    • get_option( 'wp_attachment_pages_enabled' )
  • Otherwise, redirect to the attachment URL.
Last edited 21 months ago by eatingrules (previous) (diff)

#21 in reply to: ↑ 18 ; follow-ups: @azaozz
21 months ago

Replying to SergeyBiryukov:

Yeah, that could be implemented if that's the consensus here:

  • Keep the upgrade routine from the previous iteration of the PR, which would set an option, e.g. wp_attachment_pages_enabled (to follow the link_manager_enabled precedent), so that existing sites don't need to do anything to keep the current behavior.
  • For new sites, set the wp_attachment_pages_enabled to 0 by default.

Yep, that's a pretty good way of doing it: existing sites will have the option set to true, new installs will have it set to false.

The complication there is that the option overrides the add_theme_support setting. I.e. when an existing site wants to disable attachment pages, the option will have to be changed to false (only once). This probably won't be a big problem but would need to be documented well (in the code and on make/core).

The other (simpler?) possibility would be to have a "negative" add_theme_support setting, maybe something like

add_theme_support( 'disable-attachment-pages' );
...
if ( is_attachment() && current_theme_supports( 'disable-attachment-pages' ) ) {
    // redirect
Version 0, edited 21 months ago by azaozz (next)

#22 in reply to: ↑ 21 @manfcarlo
21 months ago

Replying to azaozz:

The complication there is that the option overrides the add_theme_support setting. I.e. when an existing site wants to disable attachment pages, the option will have to be changed to false (only once). This probably won't be a big problem but would need to be documented well (in the code and on make/core).

The other (simpler?) possibility would be to have a negative add_theme_support setting, maybe something like:

add_theme_support( 'disable-attachment-pages' );
...
if ( is_attachment() && current_theme_supports( 'disable-attachment-pages' ) ) {
    // redirect

I don't like the idea of "negative" add_theme_support settings.

Why not keep it as a positive setting but apply it through core if the option is set? Then themes/plugins can remove it again using remove_theme_support instead of needing to save to the option.

if ( get_option( 'wp_attachment_pages_enabled' ) ) {
    add_theme_support( 'attachment-pages' );
}

#23 in reply to: ↑ 21 ; follow-up: @aristath
21 months ago

Yep, that's a pretty good way of doing it: existing sites will have the option set to true, new installs will have it set to false.

The complication there is that the option overrides the add_theme_support setting. I.e. when an existing site wants to disable attachment pages, the option will have to be changed to false (only once). This probably won't be a big problem but would need to be documented well (in the code and on make/core).

We could use the option to internally set theme_support, and then in our checks, use the current_theme_supports() function... So something like this:

if ( get_option( 'wp_attachment_pages_enabled' ) {
    add_theme_support( 'attachment-pages' );
}

This way, existing sites will have the feature enabled by default.
This will allow existing sites to opt-out using remove_theme_support( 'attachment-pages' ), and new sites will be able to opt-in using add_theme_support( 'attachment-pages' );

What do you think?

#24 in reply to: ↑ 23 @azaozz
21 months ago

  • Keywords needs-docs added

Replying to aristath:

We could use the option to internally set theme_support, and then in our checks, use the current_theme_supports() function... So something like this:

if ( get_option( 'wp_attachment_pages_enabled' ) {
    add_theme_support( 'attachment-pages' );
}

@aristath @manfcarlo Yep, this would work. Looking a bit more, it is pretty similar to what Sergey was talking about in comment 18. Chances are plugins and themes would probably do remove_theme_support( 'attachment-pages' ) or add_theme_support( 'attachment-pages' ) on every page load as that would be most compatible (but will be one more function call).

This also makes the "theme_support" part somewhat useless as it will be dependent on the option. So maybe WP can just use the wp_attachment_pages_enabled option and set it to true for existing sites and false for new installs. Then everybody will be able to update it as needed (hopefully just once) on plugin/theme activation.

Would still be very nice to document this option as well as possible. Actually not sure how that could be done so the inline/docblock docs would show at https://developer.wordpress.org/reference/, or if that's even possible.

Last edited 21 months ago by azaozz (previous) (diff)

#25 @SergeyBiryukov
21 months ago

  • Milestone changed from Awaiting Review to 6.3

#26 @aristath
21 months ago

Thank you for the feedback @azaozz 👍
The PR was updated to use the wp_attachment_pages_enabled option and the theme-support was removed.

#27 @matveb
20 months ago

I'm generally in favor of this proposal. Attachment pages are a bit of a clunky way to solve a usability problem. Though I do think we should review the benefits a bit, which I'd classify in two:

  • Direct links to media (permalinks).
  • Ability to explore an asset without losing the site context.

The first, particularly in a mobile world, has probably been overtaken by sharing the media asset link itself rather than the attachment permalink as an overall better experience in most sites and themes.

For the second case, which I think is now increasingly more important, I think we should explore providing a lean zoom effect on single images as a way to expand an image without having to navigate away from a page or lose context. The recent proposal for the block interactivity API could be a way forward there — check this exploration, for example, as possible path: https://github.com/WordPress/gutenberg/pull/49621

Regarding the attachment pages, it definitely seems good to retain the ability for themes to craft specific templates (photography sites and such, as mentioned before). In conjunction with the above and client transitions it could provide, in the future, a sort of custom lightbox experience controlled by the theme and powered by attachment templates, but I digress a bit.

Back tot he implementation, I'm not entirely sure about add_theme_support(), particularly in the context of block themes which generally don't need to use functions.php or work it through theme.json directly. Would it make more sense to make the support implicit by adding one of the relevant templates in the hierarchy? The site option would still be honored, but the theme doesn't need to do anything extra outside of supplying the template to make itself ready. Same for a user creating a custom template for media.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


20 months ago

#29 @antpb
20 months ago

In the recent Media meeting we discussed this issue and paths forward. While I agree with the intent of using the template hierarchy to determine if folks are using attachement pages, it assumes a bit too much considering many can do things like have conditional checks for an attachment in their single.php file which would cause a breaking change if we made any adjustment to attachements. They would then need to build an attachment.php file to adapt.

If we keep the scope of this issue focused on adding an add_theme_support() we could later change that default with more communication to potentially impacted sites. This gives us a middle ground to move toward the future state we all want and potentially even allows us to explore using the theme hierarchy to determine if enabled/disabled.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


19 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


18 months ago

#33 @antpb
18 months ago

  • Milestone changed from 6.3 to 6.4

This is a pretty big change to land at this point in 6.3 we should aim to prioritize a decision on the way to solve this out early in 6.4. Moving to that release.

#34 @joostdevalk
18 months ago

We shouldn't keep on pushing this back though, that's too easy. This is literally causing SEO issues for every WordPress site out there right now that's not running an SEO plugin.

I'm perfectly fine with a solution that leaves all existing sites unaffected unless they do something themselves, and disables these pages by default for all new sites... That should be acceptable to everyone I think?

#35 @oglekler
16 months ago

  • Keywords needs-refresh added

@aristath I was unable to apply the patch, I think it needs to be rebased. 

Otherwise, from my point of view, this change looks logical, and I don't see any blockers for this behaviour:

  1. Keep current state for existing sites
  2. Disable attachments pages for new installation
  3. Provide a way for developers to enable attachment pages in their themes. It also means new docs :)

#36 @aristath
16 months ago

Thank you @oglekler!

I rebased the PR, and I also moved the upgrade from upgrade_630 to upgrade_640. I used a dummy version number for the $wp_current_db_version and set it to 57000 for the sake of this patch. The DB version is something that changes on release, so not a blocker.

#37 @oglekler
16 months ago

  • Keywords needs-testing added; 2nd-opinion needs-refresh removed

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


16 months ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


16 months ago

This ticket was mentioned in Slack in #core by oglekler. View the logs.


15 months ago

This ticket was mentioned in Slack in #core-test by oglekler. View the logs.


15 months ago

@zunaid321
15 months ago

This plugin adds an admin bar item to toggle the new option. Created by @costdev

#42 @zunaid321
15 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/4233

Environment

  • OS: Windows 11 (22H2)
  • Web Server: nginx/1.25.1
  • PHP: 7.4.33
  • WordPress: 6.4-alpha-56267-src
  • Browser: Chrome Version 113.0.5672.126 (Official Build) (64-bit)
  • Theme: Twenty Twenty-Three (V1.2)
  • Plugins: 57913-toggle-wp-attachment-pages-enabled.zip

Instructions

  • Apply the patch
  • Upload and activate the attached plugin
  • Navigate to Media > Library.
  • Upload an image
  • Click on a media item, then click View attachment page. Expected: An attachment page should load.
  • Click the Disable Attachment Pages link in the admin bar.
  • Refresh the page. Expected: The media file should be directly loaded instead of an attachment page.
  • Navigate back to WordPress administration area and click Enable Attachment Pages to restore the original value.

Actual Results

  • ✅ With wp_attachment_pages_enabled enabled: The attachment page continues to load as normal.
  • ✅ With wp_attachment_pages_enabled disabled: The attachment file is loaded directly for plain and numeric permalinks.
  • ❌ With wp_attachment_pages_enabled disabled: The attachment page loads for non-plain/numeric permalinks.

Notes

The patch works for plain and numeric permalinks, but not for the other permalink structure presets or a custom permalink structure.

  • For Plain and Numeric ✅
  • For Other Permalink Structures (Including Custom Structure with all the available tags) ❌

Supplemental Artifacts

For Plain and Numeric: https://i.imgur.com/PmFdrXr.png
For Other Permalink Structures: https://i.imgur.com/TghCSlg.png

Special thanks to @costdev for providing the instructions and the custom plugin!

#43 @costdev
15 months ago

  • Keywords has-testing-info added

#44 @SergeyBiryukov
15 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#45 @aristath
15 months ago

Thank you @zunaid321 for the test!

Pushed a fix, tested all permalink structures and it now works as expected.
Props @SergeyBiryukov @poena @afercia @aristath, this was done during a team coding session.

#46 @SergeyBiryukov
15 months ago

To summarize a bit, I believe all the feedback above has been addressed.

The current implementation is along the lines of comment:18, minus the unnecessary add_theme_support() feature, as per comment:24:

  • An upgrade routine sets the wp_attachment_pages_enabled option to 1, so that existing sites don't need to do anything to keep the current behavior. This follows the link_manager_enabled precedent.
  • For new sites, wp_attachment_pages_enabled is set to to 0 by default.
  • Older sites that want to disable the attachment pages can also set the option to 0.
  • If get_option( 'wp_attachment_pages_enabled' ) returns a truthy value, attachment pages work as is.
  • Otherwise, attachment pages are redirected to the attachment URL.

#47 @SergeyBiryukov
15 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 56657:

Media: Disable attachment pages for new installations.

WordPress creates attachment pages by default for every attachment uploaded. On the vast majority of sites, these attachment pages don't contain any meaningful information. They do however exist, get indexed by search engines, and sometimes even rank in search results, leading to bad results for users and site owners.

This commit introduces a wp_attachment_pages_enabled database option to control the attachment pages behavior:

  • On existing sites, the option is set to 1 on upgrade, so that attachment pages continue to work as is.
  • For new sites, the option is set to to 0 by default, which means attachment pages are redirected to the attachment URL.
  • Sites that want to enable or disable the attachment pages can set the option to 1 or 0, respectively.

Follow-up to [2958], [3303], [7149], [34690].

Props aristath, poena, afercia, joostdevalk, jonoaldersonwp, azaozz, johnbillion, joedolson, basiliskan, audrasjb, davelo, rilwis, manfcarlo, tyxla, garrett-eclipse, seedsca, eatingrules, matveb, antpb, zodiac1978, oglekler, zunaid321, costdev, SergeyBiryukov.
Fixes #57913.

@SergeyBiryukov commented on PR #4233:


15 months ago
#48

Thanks for the PR! Merged in r56657.

#49 @SergeyBiryukov
15 months ago

In 56658:

Media: Use correct option name for attachment pages in upgrade_640().

Follow-up to [56657].

See #57913.

#50 @SergeyBiryukov
15 months ago

  • Keywords needs-dev-note added

#51 @joedolson
15 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

When attachment pages are disabled, the links to 'View attachment page' should change to 'View Media File' or something equivalent.

@joedolson
15 months ago

Change text links based on attachment page status

This ticket was mentioned in Slack in #core by oglekler. View the logs.


15 months ago

#53 @oglekler
15 months ago

This ticket was discussed during a bug scrub,

The additional patch needs testing before it will be commited.

Add props to @hellofromtonya

@ironprogrammer
15 months ago

Refreshed patch to update option to wp_attachment_pages_enabled.

#54 @ironprogrammer
15 months ago

Thanks for the patch, @joedolson! The refreshed patch 57913.3.diff updates the option name to wp_attachment_pages_enabled per comment:47. Test report forthcoming.

#55 @ironprogrammer
15 months ago

Test Report

Patch tested: 57913.3.diff refresh

Environment

  • Hardware: MacBook Pro Apple M1 Pro
  • OS: macOS 13.5.2
  • Browser: Safari 16.6
  • Server: nginx/1.25.2
  • PHP: 7.4.33
  • WordPress: 6.4-alpha-56267-src

Actual Results

  • ✅ From the media library list view: "Edit Media" (e.g. /wp-admin/post.php?post=1262&action=edit), the admin bar's view media link changes to reflect the status of wp_attachment_pages_enabled .
  • ✅ From the media library grid view: "Attachment details" (e.g. /wp-admin/upload.php?item=1262), the attachment action link reflects the status of wp_attachment_pages_enabled .

Additional Information

  • Used wp option set wp_attachment_pages_enabled 0|1 to toggle the attachment pages setting.
  • The different views are accessed via the list or grid options:

https://cldup.com/r1fb1Wc3GL.thumb.jpg

Supplemental Artifacts

Figure 1: Edit Media admin bar
https://cldup.com/qh6ThQxz0j.gif

Figure 2: Attachment details action link
https://cldup.com/jo0XJtDO97.gif

#56 @hellofromTonya
15 months ago

  • Keywords commit added; needs-docs needs-testing removed

Thanks @joedolson for catching that issue and @ironprogrammer for updating the patch and testing to confirm what Joe found.

Marking 57913.3.diff for commit.

Also cleared past keywords that seem to be resolved prior to [56657].

#57 @hellofromTonya
15 months ago

Prepping the commit now.

#58 @hellofromTonya
15 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56711:

Media: Change link text when attachment pages disabled.

When attachment pages are disabled, change the links from "View Attachment Page" to "View Media File".

Follow-up to [56657], [56658].

Props joedolson, ironprogrammer, oglekler.
Fixes #57913.

#59 @webcommsat
14 months ago

@SergeyBiryukov we are reviewing dev notes marked up with needs dev-notes. Do you or anyone else working on this ticket have a draft of a dev note please?
The dev note for review and publishing purposes is being tracked at this [link]https://github.com/WordPress/Documentation-Issue-Tracker/issues/1166[].
Thanks for your help.

Update: in our docs discussion, we thought your [[comment 47]https://core.trac.wordpress.org/ticket/57913?cnum_edit=59#comment:47] and from some of the previous discussion, that this dev note might also benefit from a call out in the Field Guide. Your thoughts welcome on this. Thanks

Last edited 14 months ago by webcommsat (previous) (diff)

#60 @joostdevalk
14 months ago

I'll pick up writing this dev note.

#61 @webcommsat
14 months ago

  • Keywords add-to-field-guide added; has-patch has-testing-info removed

This will now go into the Field Guide under the Media component.

Also highlighting for @joostdevalk to marketing as a big win for SEO - will share this ticket in the Marcomms release channel.

Removed other workflow tickets no longer relevant for housekeeping.

Update: for completeness, the published [dev note link](https://make.wordpress.org/core/2023/10/16/changes-to-attachment-pages/)

Last edited 14 months ago by webcommsat (previous) (diff)

#62 follow-up: @wpsunshine
14 months ago

A little late to this one, but I think a filter to allow/disallow attachment page on a per attachment basis is even more ideal instead of a global setting. You could have 10k attachments you don't want a unique page for, but may want to make use of it for a subset.

An example is my Sunshine Photo Cart plugin which relies on each image having it's own attachment page for selling but I can see portfolio images on the same site not needing them.

Last edited 14 months ago by wpsunshine (previous) (diff)

#64 @sabernhardt
14 months ago

  • Keywords has-dev-note added

#65 in reply to: ↑ 62 ; follow-up: @azaozz
14 months ago

Replying to wpsunshine:

An example is my Sunshine Photo Cart plugin which relies on each image having it's own attachment page

This change will affect only new sites created with WP 6.4+. Perhaps add checking/resetting of the option to your plugin when updating it?

#66 in reply to: ↑ 65 ; follow-up: @wpsunshine
14 months ago

Replying to azaozz:

Replying to wpsunshine:

An example is my Sunshine Photo Cart plugin which relies on each image having it's own attachment page

This change will affect only new sites created with WP 6.4+. Perhaps add checking/resetting of the option to your plugin when updating it?

That was the first thing I did when I saw this ticket :) But not every attachment on a website using my plugin needs it's own URL - only the ones in my plugin - and setting a global option is not helping a site like this.

While I overall agree with the goal of this ticket to reduce the number of unnecessary URLs for attachments that definitely don't need one, there are many things in the WP ecosystem that do rely on attachment pages. Having a filter (which defaults to the option value) to allow/disallow on a per-attachment basis I think would make this an even better way to go about implementing this change instead of only a global option.

Last edited 14 months ago by wpsunshine (previous) (diff)

#67 in reply to: ↑ 66 @hellofromTonya
14 months ago

Replying to wpsunshine:

While I overall agree with the goal of this ticket to reduce the number of unnecessary URLs for attachments that definitely don't need one, there are many things in the WP ecosystem that do rely on attachment pages. Having a filter (which defaults to the option value) to allow/disallow on a per-attachment basis I think would make this an even better way to go about implementing this change instead of only a global option.

Hello @wpsunshine, Thank you for suggestion. With 6.4 RC1 tomorrow and given that adding a new filter is an enhancement, it's too late in the 6.4 cycle.

Can you open a new Trac ticket please? Adding a new filter could be considered during the 6.5 cycle.

#68 follow-up: @joppuyo
13 months ago

Hey! I'm an author of a plugin Disable Media Pages which has provided this functionality in previous WordPress versions. I'm trying to figure out what's the best way forward for the plugin since WordPress 6.4 as this functionality is now built into the core.

While testing 6.4, I noticed that even if attachment_pages_enabled is set to 0 the redirection only kicks in for logged-in users. For anonymous users, the attachment page is still displayed. It seems this is because read_post capability is checked before the redirection is performed. Anonymous users don't have any capabilities so the redirection work in this case.

Is this feature intended to work this way or is this an oversight in the implementation?

Last edited 13 months ago by joppuyo (previous) (diff)

#69 in reply to: ↑ 68 ; follow-up: @chesio
13 months ago

Replying to joppuyo:

While testing 6.4, I noticed that even if attachment_pages_enabled is set to 0 the redirection only kicks in for logged-in users. For anonymous users, the attachment page is still displayed. It seems this is because read_post capability is checked before the redirection is performed. Anonymous users don't have any capabilities so the redirection work in this case.

Is this feature intended to work this way or is this an oversight in the implementation?

I observed the same behaviour and I believe it's an oversight. The initial idea was to prevent such pages from being crawled and indexed by search engines, but this is obviously not the case.

But since this ticket is closed and the feature has been shipped with 6.4 already, I think creating a new ticket is the best way to proceed now.

#70 in reply to: ↑ 69 @joppuyo
13 months ago

Replying to chesio:

I observed the same behaviour and I believe it's an oversight. The initial idea was to prevent such pages from being crawled and indexed by search engines, but this is obviously not the case.

But since this ticket is closed and the feature has been shipped with 6.4 already, I think creating a new ticket is the best way to proceed now.

I created a new ticket #59866 to report this bug

#71 @aristath
13 months ago

Thank you @joppuyo!
@afercia already has a patch for this one, we were working on it just now so the patch will be submitted shortly

#72 @peterwilsoncc
11 months ago

In 57310:

Media: Redirect inactive attachement pages for logged-out users.

Ensure logged out users are redirected to the media file when attachment pages are inactive. This removes the read_post capability check from the canonical redirects as anonymous users lack the permission.

Follow-up to [56657], [56658], [56711].

Props afercia, aristath, chesio, joppuyo, jorbin, lakshmananphp, poena, sergeybiryukov.
Fixes #59866.
See #57913.

#73 @peterwilsoncc
11 months ago

In 57318:

Media: Revert [57310].

This commit reintroduced a minor data exposure issue.

Props swissspidy.
See #59866, #57913.

#74 @jorbin
11 months ago

In 57357:

Media: Redirect inactive attachment pages for logged-out users.

Ensure logged out users are redirected to the media file when attachment pages are inactive. This removes the read_post capability check from the canonical redirects as anonymous users lack the permission.

This was previously committed in [57310] before being reverted in [57318]. This update includes a fix to cover instances where revealing a URL could be considered a data leak and greatly expands the unit tests to ensure that this is covered along with many other instances.

Follow-up to [56657], [56658], [56711], [57310], [57318].

Props peterwilsoncc, jorbin, afercia, aristath, chesio, joppuyo, jorbin, lakshmananphp, poena, sergeybiryukov, swissspidy, johnbillion.
Fixes #59866.
See #57913.

#75 @jorbin
11 months ago

In 57358:

Media: Redirect inactive attachment pages for logged-out users.

Ensure logged out users are redirected to the media file when attachment pages are inactive. This removes the read_post capability check from the canonical redirects as anonymous users lack the permission.

This was previously committed in [57310] before being reverted in [57318]. This update includes a fix to cover instances where revealing a URL could be considered a data leak and greatly expands the unit tests to ensure that this is covered along with many other instances.

Follow-up to [56657], [56658], [56711], [57310], [57318].

Reviewed by joemcgill.
Merges [57357] to 6.4 branch.

Props peterwilsoncc, jorbin, afercia, aristath, chesio, joppuyo, jorbin, lakshmananphp, poena, sergeybiryukov, swissspidy, johnbillion, mukesh27.
Fixes #59866.
See #57913.

#76 @Mamaduka
8 months ago

I want to highlight the discussion from the Gutenberg repo: https://github.com/WordPress/gutenberg/issues/56019.

Should the /media endpoint account for this flag and omit the link value from the response when the attachment pages are disabled?

This would allow media blocks to hide the "Link to attachment page" link control option.

An alternative solution would be to expose the setting via REST API, but the endpoint is only accessible for users with manage_options capabilities.

Note: See TracTickets for help on using tickets.