w3guy / persist-admin-notices-dismissal

Simple plugin that persists dismissal of admin notices across pages in WordPress dashboard.
http://w3guy.com/wordpress-admin-notices-dismissible/
87 stars 22 forks source link

WPINC is not defined. Plugin causes white screen. #14

Closed dac514 closed 6 years ago

dac514 commented 6 years ago

The first 3 lines of this plugin do:

if ( ! defined( 'WPINC' ) ) {
    die;
}

If you look at the WordPress Code you can see that WPINC is maybe defined in the 3rd condition. We are setup using https://roots.io/bedrock/ and this plugin is autoloaded using Composer, therefore WPINC is never defined and die causes a "white screen of death"

Please use ABSPATH instead.

afragen commented 6 years ago

WPINC is defined in wp-settings.php and should be called in any standard WP installation.

https://github.com/WordPress/wordpress-develop/blob/8f95800d52c1736d651ae6e259f90ad4a0db2c3f/src/wp-settings.php#L16

Besides, this is simply to prevent the file from being called directly.

dac514 commented 6 years ago

@afragen

Becauase the plugin is loaded using Composer Autoload, because we use Bedrock, because of the reasons above... WPINC is not always defined.

ABSPATH does the same thing.

@see https://wordpress.stackexchange.com/questions/108418/what-are-the-differences-between-wpinc-and-abspath

afragen commented 6 years ago

You have a number of moving pieces present.

Having a file make use of Composer’s autoloader does not call the file directly, it allows the class to be autoloaded if/when someone calls the class eliminating the need for a require or include command.

I confess I no nothing of Bedrock, but I do know that in the usual manner of running WordPress the wp-settings.php file is called and WPINC is defined.

All this said, I don’t have a strong feeling one way or another and I’ll wait for @collizo4sky to weigh in, but my gut tells me something in Bedrock is loading WP without the wp-settings.php file.

dac514 commented 6 years ago

Bedrock might not be standard, but it is best practices for web development IMHO. The issue is that the bootstrapping starts here:

https://github.com/roots/bedrock/blob/master/web/wp-config.php

This plugin, being Composer compatible (2018, yay!), will get initialized by `vendor/autoload.php' before wp-settings.php is called.

dac514 commented 6 years ago

Xdebug trace:

xdebug-trace

Thank you for your consideration.

dac514 commented 6 years ago

@afragen I'm going to open an issue on Bedrock. I looked at your plugins. The GitHub one uses this convention as well. From what you say, seems like this convention is common and maybe this is a Bedrock issue. Thanks for the info.

dac514 commented 6 years ago

Related: https://github.com/roots/bedrock/issues/343

afragen commented 6 years ago

Fixed. https://github.com/collizo4sky/persist-admin-notices-dismissal/commit/067589be4c0b9dfa221320dfb15f9752269c3c82

composer update :wink:

w3guy commented 6 years ago

I had this issue in my ProfilePress plugin. I had to fix it by adding this line define( 'WPINC', true );

I just need to update the code to define( 'ABSPATH', true ); because ABSPATH is not defined in my custom bootstrap process.

Thanks @afragen for the fix for bedrock users