Make WordPress Core

Opened 12 years ago

Closed 4 years ago

Last modified 4 years ago

#22101 closed defect (bug) (fixed)

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

Reported by: ifrins's profile ifrins Owned by: iworks's profile iworks
Milestone: 5.5 Priority: normal
Severity: normal Version: 3.4.2
Component: Feeds Keywords: good-first-bug has-patch has-unit-tests commit
Focuses: Cc:

Description

Hi,
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 12 years ago.
Page HTML Output
feed-output.xml (2.0 KB) - added by ifrins 12 years ago.
Feed Output
22101.gallery.link.in.feed.diff (810 bytes) - added by iworks 9 years ago.
22101.2.diff (1.1 KB) - added by stevenkword 7 years ago.
Logic revisions @ 4.7.4
22101.3.patch (1.8 KB) - added by birgire 7 years ago.

Download all attachments as: .zip

Change History (25)

@ifrins
12 years ago

Page HTML Output

@ifrins
12 years ago

Feed Output

#1 @ifrins
12 years ago

  • Cc francesc.bgr+ifrins@… added

#2 follow-up: @mdgl
12 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
12 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
12 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
9 years ago

  • Keywords needs-patch needs-testing added

#6 @stevenkword
9 years ago

  • Keywords good-first-bug added

#7 @iworks
9 years ago

It ignore whole attribute "link". Patch attached.

#8 @DrewAPicture
8 years ago

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

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

#9 @DrewAPicture
8 years ago

  • Keywords has-patch added; needs-patch removed

@stevenkword
7 years ago

Logic revisions @ 4.7.4

#10 @stevenkword
7 years 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"]

https://codex.wordpress.org/Gallery_Shortcode#Options

Last edited 7 years ago by stevenkword (previous) (diff)

@birgire
7 years ago

#11 @birgire
7 years 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 7 years ago by birgire (previous) (diff)

#12 @birgire
7 years 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 7 years ago by birgire (previous) (diff)

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


4 years ago

#14 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.5

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


4 years ago

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


4 years ago

#17 @SergeyBiryukov
4 years ago

  • Keywords commit added

#18 @whyisjake
4 years ago

Looks good to me, just needed to swap the test to check for .jpg instead of .png.

#19 @whyisjake
4 years ago

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

In 48496:

Feeds: Ensure that galleries can be output as a list of links in feeds.

Adjusts the gallery shortcode handler to check for the link attribute when outputting to a feed.

Fixes #22101.

Props ifrins, mdgl, SergeyBiryukov, chriscct7, stevenkword, iworks, DrewAPicture, birgire, whyisjake.

#20 @SergeyBiryukov
4 years ago

In 48516:

Tests: Simplify some assertions in phpunit/tests/media.php.

Correct comments per the documentation standards.

Follow-up to [48496].

See #22101.

Note: See TracTickets for help on using tickets.