Closed raamdev closed 7 years ago
Here's L1026 on the dev branch that's throwing the Array to string conversion notice: https://github.com/websharks/comment-mail-pro/blob/000000-dev/src/includes/classes/SubInserter.php#L1026
It looks like $this->sub->post_id
is an Array for some reason, but probably shouldn't be.
Confirmed via XDebug that $this->sub->post_id
is an Array:
It looks like this line that calls SubEventLogInserter
is modifying $this->sub->post_id
and turning it into an array.
The SubEventLogInserter
is calling the following:
$this->plugin->utils_db->wp->insert($this->plugin->utils_db->prefix().'sub_event_log', $this->entry)
That eventually calls the WordPress process_fields()
function with the data (i.e., $this->entry
aka $this->sub
), which is resulting in the data in $this->sub
being modified unexpectedly.
So it sounds like this line should be passing a copy of $this->sub
to the SubEventLogInserter
, so that we don't end up with unexpected data.
@jaswsinc do you agree?
Yes. I agree. You found a PHP 🐛 it looks like! :-)
Confirmed that this issue does not occur with PHP 5.5. I'm marking this a php bug
and moving this issue to the Future Release milestone. PHP7 support is not a top priority at the moment (getting the next release out is a top priority, as its been 4 months since the last release).
Noting that this issue also affects the StCR importer.
Marking this as a high-priority issue to solve one the current release cycle finishes. As it stands, Comment Mail is unusable on PHP 7, so we either need to detect sites running PHP 7 and provide a Dashboard notice, or fix the underlying issue so that it's not an issue.
Objective: Confirm via XDebug that this does not impact prior versions of PHP and also test against the latest release of PHP v7.0.x and 7.1 to see if the problem has been solved; assuming it is in fact a PHP v7. x bug. ~ Estimate at 1 day.
Referencing the origin of the unexpected array of data noted above. https://github.com/WordPress/WordPress/blob/4.6.1/wp-includes/wp-db.php#L2161-L2215
Confirmed this is in fact a PHP 7 bug.
This bug appears to have been corrected in PHP 7.0.9. https://bugs.php.net/bug.php?id=72508
And then another related bug was corrected in PHP 7.0.10. https://bugs.php.net/bug.php?id=72622
Also referencing: http://php.net/ChangeLog-7.php
Conclusion: Comment Mail requires PHP 5.4.x+, or when upgrading to PHP 7, requires PHP 7.0.9+ with PHP 7.0.10 being suggested as it includes some additional bug fixes that are indirectly related to a problem with PHP internals creating references inadvertently.
Next Release Changelog:
This issue can be closed by @raamdev once a review is complete.
@jaswsinc Awesome! Glad we got to the bottom of that issue. In your screenshot, that notice is dismissable, but IMO that notice should be persistent, since a site owner could inadvertently dismiss it and might then wonder why Comment Mail isn't working as expected. Comment Mail is truly broken (notifications don't get sent) when running PHP 7 < 7.0.9, so I think a persistent notice is applicable here.
Agree. I'm opening a fork in https://github.com/websharks/comment-mail/issues/308
No longer dismissable. Fixed in https://github.com/websharks/comment-mail/issues/308
Comment Mail v161118 has been released and includes changes from this GitHub Issue. See the v161118 announcement for further details.
This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#272).
When leaving a reply on a comment and choosing to subscribe to replies to my comment, I'm getting the following notice (tested with the latest Comment Mail Pro on the
000000-dev
branch; also had this happen with the latest public release of Comment Mail Lite v160213).Despite this notice, the subscription does appear to get created in Comment Mail, however the status says
unconfirmed
, despite Auto-Confirm Everyone enabled in Comment Mail → Config. Options → Auto-Confirm Settings (is that perhaps a separate bug?).Errors when testing the dev branch on 2016-06-08:
Errors testing latest public release (v160215):
Related: https://github.com/websharks/comment-mail/issues/298