Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account for filtering of gallery 'link' attribute #3439

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Oct 3, 2019

Summary

This fixes a case where a gallery lightbox doesn't open, when adding a $atts['link'] = 'file' to a 'shortcode_atts_gallery' filter.

Addresses an edge case from issue #2850 (comment)

Steps to reproduce

  1. Create a post
  2. Add a Shortcode block
  3. Enter a gallery, like:
[gallery ids="3762, 3470, 3327, 2877"]
  1. Select 'Add lightbox effect'
  2. Publish the post, and go to the front-end
  3. Add this to a plugin:
add_filter(
	'shortcode_atts_gallery',
	function( $atts ) {
		$atts['link'] = 'file';
		return $atts;
	}
);
  1. Reload the front-end
  2. Expected: on clicking an image in the gallery, it opens a lightbox
  3. Actual: it goes to the URL of the image:

goes-to-url-of-image

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

This usually worked,
but it's still possible to set the
attributes via the filter 'shortcode_atts_gallery'.
So there was a user that relied on that filter,
and that overwrote the previous link="file" value,
and made it 'none'.
So this adds a filter at a high priority.
@googlebot googlebot added the cla: yes Signed the Google CLA label Oct 3, 2019
Comment on lines -177 to +183
$attributes['link'] = 'none';
$html = gallery_shortcode( $attributes );
$html = gallery_shortcode( $attributes );
Copy link
Contributor Author

@kienstra kienstra Oct 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When gallery_shortcode() runs, it's possible to filter the $attributes with the shortcode_atts_gallery filter.

And that can change the previous value of $attributes['link'] = 'none';, causing the lightbox to not work.

@kienstra
Copy link
Contributor Author

kienstra commented Oct 4, 2019

Request For Review

Hi @westonruter,
Hope your Friday's off to a great start.

You've probably looked at the code at least a little, but could you please do a formal review?

It looks like this PR works for @rsmith4321:

I also tested the latest update and it seems to fix the issue with $attr['link'] = 'file';. I've added that back into my functions.php and the lightbox is working, thanks.

Thanks, Weston! Have a great weekend.

@kienstra kienstra requested a review from westonruter October 4, 2019 17:05
@westonruter westonruter merged commit 53e18d0 into develop Oct 4, 2019
@westonruter westonruter deleted the update/gallery-link-attribute branch October 4, 2019 17:22
@kienstra
Copy link
Contributor Author

kienstra commented Oct 4, 2019

Thanks, Weston!

@swissspidy swissspidy modified the milestones: v1.3.1, v1.4 Oct 17, 2019
@kienstra
Copy link
Contributor Author

kienstra commented Oct 28, 2019

Question about moving to 'Ready for Merging'

Hi @westonruter,
Hope you had a great weekend.

Would it be alright to move this to 'Ready for Merging,' without traditional QA? The steps to reproduce include adding a PHP filter.

Thanks, Weston!

@westonruter
Copy link
Member

Given it was already tested by the other Ryan, then yes, I moved it.

@kienstra
Copy link
Contributor Author

Thanks, Weston!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants