Skip to content

Fix notices from an undefined post variable#104

Open
srtfisher wants to merge 5 commits intoAutomattic:developfrom
alleyinteractive:master
Open

Fix notices from an undefined post variable#104
srtfisher wants to merge 5 commits intoAutomattic:developfrom
alleyinteractive:master

Conversation

@srtfisher
Copy link
Copy Markdown

Fixes notices from the shortcode when used on taxonomy pages (which do not have a $post set):

PHP message: Warning: Attempt to read property "ID" on null in /var/www/wp-content/plugins/polldaddy/polldaddy-shortcode.php on line 127

Changes proposed in this Pull Request:

Testing instructions:

Screenshot / Video

Proposed changelog entry for your changes:

@jgcaruso
Copy link
Copy Markdown
Contributor

Hey there, thanks for the contribution and sorry for the delay in a response.

While the code change itself looks reasonable (and safer), would you mind providing testing instructions? I'm having trouble reproducing the original issue on the main branch. Pages set with a custom taxonomy seem to be working fine for me. Are you doing something differently? Is it on the taxonomy preview page and the full page content is being displayed instead of a summary?

My main concern here is I'd like to provide defaults for the values that are not having any values set (ex $unique_id)

Also, I don't think we want to introduce a composer.json, so would you mind excluding it from the PR? Thanks!

@GaryJones GaryJones changed the base branch from master to develop September 22, 2025 17:36
@srtfisher
Copy link
Copy Markdown
Author

@jgcaruso because $post is a global it could be null and not WP_Post. Assuming it is WP_Post is what causes notices for us on NYPost.

@jgcaruso
Copy link
Copy Markdown
Contributor

@jgcaruso because $post is a global it could be null and not WP_Post. Assuming it is WP_Post is what causes notices for us on NYPost.

Hey there, just want to let you know I'm no longer with Automattic so I don't have merge privileges anymore. Wish I could be more helpful here, but I don't know who to direct this to now.

@srtfisher
Copy link
Copy Markdown
Author

@jgcaruso thanks for the heads up! I figure somebody someday will get to it. Perhaps @GaryJones ?

@GaryJones GaryJones self-assigned this Nov 27, 2025
@GaryJones
Copy link
Copy Markdown
Contributor

Hey @srtfisher, thanks for this contribution and for your patience while this PR has been waiting!

The fix makes sense - on taxonomy archives and other contexts where $post is null, accessing $post->ID will indeed throw warnings.

I do want to flag one consideration before merging: with these guard clauses, when $post is null, the $settings array (lines 143-149) will end up with empty values for unique_id, item_id, title, and permalink. I'm not sure how the Crowdsignal rating widget handles this - it might work fine, or it might result in ratings that can't be properly identified/tracked.

A couple of options come to mind:

  1. Early return - If $post is null and the shortcode doesn't provide explicit unique_id/item_id attributes, return a comment like <!-- Crowdsignal rating requires post context --> instead of rendering a potentially broken widget.
  2. Fallback defaults - Generate fallback identifiers when $post is null (e.g., based on the current URL or a hash), so the rating can still function

What's your experience been on NYPost? Are you using the rating shortcode on taxonomy pages with explicit unique_id and item_id attributes, or does it work okay with empty values? That would help us understand which approach makes most sense here.

Thanks again for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants