Make WordPress Core

Opened 5 years ago

Last modified 4 weeks ago

#22101 assigned defect (bug)

Gallery shortcode with link="file" is not linking the file in the RSS feed

Reported by: ifrins Owned by: iworks
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.4.2
Component: Feeds Keywords: good-first-bug has-patch has-unit-tests
Focuses: Cc:


In a WordPress site I'm currently developing I want to use the stock gallery shortcode. I use [gallery link="file"] to link directly the image URL which, should fix Flipboard, Currents et al. showing just a little thumbnail.

In the page the code is displayed properly and the a tag links the image file, but, on the RSS feed, it links to the attachment page.

I've tested this in the latest nightly version 3.5-beta1-22104 and on the stable 3.4.2. I attach the output of the page and the feed.

Attachments (5)

page-output.html (2.7 KB) - added by ifrins 5 years ago.
Page HTML Output
feed-output.xml (2.0 KB) - added by ifrins 5 years ago.
Feed Output
22101.gallery.link.in.feed.diff (810 bytes) - added by iworks 21 months ago.
22101.2.diff (1.1 KB) - added by stevenkword 6 months ago.
Logic revisions @ 4.7.4
22101.3.patch (1.8 KB) - added by birgire 4 weeks ago.

Download all attachments as: .zip

Change History (17)

5 years ago

Page HTML Output

5 years ago

Feed Output

#1 @ifrins
5 years ago

  • Cc francesc.bgr+ifrins@… added

#2 follow-up: @mdgl
5 years ago

This happens because for some reason the gallery shortcode deliberately generates completely different output for feeds. In file wp-includes/media.php the function gallery_shortcode() contains the following lines:

if ( is_feed() ) {
        $output = "\n";
        foreach ( $attachments as $att_id => $attachment )
                $output .= wp_get_attachment_link($att_id, $size, true) . "\n";
        return $output;

Why it would want to do this is anybody's guess! Maybe it's just a hangover from the days of the limited functionality of early feed readers. In any case, as I hope you can see, the code doesn't reference the gallery link option at all.

There appear to be two options to fix the problem. Firstly, we could just delete the block of code above and allow feed output to contain the full gallery HTML/CSS as though it was a normal page. Alternatively, we could modify the code above to respect the value of the gallery link option, something like the following (note this has not been tested - it's just a suggestion, but based on similar code that occurs later on in the gallery shortcode function):

if ( is_feed() ) {
        $output = "\n";
        $permalink = !isset($attr['link']) || 'file' != $attr['link'];
        foreach ( $attachments as $att_id => $attachment )
                $output .= wp_get_attachment_link($att_id, $size, $permalink) . "\n";
        return $output;

What do you think? What should be the correct behaviour of the gallery within feeds?

#3 in reply to: ↑ 2 ; follow-up: @SergeyBiryukov
5 years ago

Replying to mdgl:

Why it would want to do this is anybody's guess!

Introduced in [7298] (for #6225).

#4 in reply to: ↑ 3 @mdgl
5 years ago

Replying to SergeyBiryukov:

Introduced in [7298] (for #6225).

Ah yes, the world was a different place in 2008!! Including perhaps the ability to get away with a one-line bug report containing no example or explanation :-)

#5 @chriscct7
2 years ago

  • Keywords needs-patch needs-testing added

#6 @stevenkword
22 months ago

  • Keywords good-first-bug added

#7 @iworks
21 months ago

It ignore whole attribute "link". Patch attached.

#8 @DrewAPicture
17 months ago

  • Owner set to iworks
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

#9 @DrewAPicture
17 months ago

  • Keywords has-patch added; needs-patch removed

6 months ago

Logic revisions @ 4.7.4

#10 @stevenkword
5 months ago

22101.2.diff updates the logic to account for the following cases

Default: links to image attachment page url
[gallery ids="5,1464"]

URL: Links to image file url
[gallery link="http://src.wordpress-develop.dev/wp-content/uploads/2017/04/IMG_20150829_100814.jpg"]

None: Does not link
[gallery ids="5" link="none"]

File: Links to image file url
[gallery ids="5" link="file"]


Last edited 5 months ago by stevenkword (previous) (diff)

4 weeks ago

#11 @birgire
4 weeks ago

  • Keywords has-unit-tests added; needs-testing removed

Added a test in 22101.3.patch for the cases mentioned by @stevenkword (apart from the link attribute as an url, as it's not supported afaik).

As unrelated, I noticed that the test in test_wp_get_attachment_image_should_use_wp_get_attachment_metadata() can only run once, because the filename gets an increasing index, like test-image-large-{$i}-150x150.png in the srcset, where $i > 2 is the number of times we run the /tests/phpunit/tests/media.php tests. Looks like we might need a better cleanup there?

I tried to avoid this in the above test by not referring to the exact filename for self::$large_id, as it would also get the same increasing index for repeated test runs.

Last edited 4 weeks ago by birgire (previous) (diff)

#12 @birgire
4 weeks ago

Regarding the missing attachment cleanup for tests in media.php, that I stumbled on yesterday, see #38264 (it's being addressed there)

Last edited 4 weeks ago by birgire (previous) (diff)
Note: See TracTickets for help on using tickets.