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

Fix PHP 8.x fatal error upon saving custom admin color scheme #34

Open
KZeni opened this issue May 30, 2023 · 1 comment
Open

Fix PHP 8.x fatal error upon saving custom admin color scheme #34

KZeni opened this issue May 30, 2023 · 1 comment

Comments

@KZeni
Copy link
Contributor

KZeni commented May 30, 2023

It seems the lib/phpsass library (last updated 5 years ago, it seems) needs to be updated to a new version or otherwise patched to avoid a fatal PHP error when saving a color scheme for those using PHP 8.0+.

The exact errors appear to be:

  1. Fatal error: An error of type E_COMPILE_ERROR was caused in line 133 of the file /wp-content/plugins/admin-color-schemer/lib/phpsass/tree/SassRuleNode.php. Error message: Array and string offset access syntax with curly braces is no longer supported
  2. Fatal error: Uncaught Error: count(): Argument #1 ($value) must be of type Countable|array, string given in /wp-content/plugins/admin-color-schemer/lib/phpsass/tree/SassMixinNode.php on line 66
  3. Fatal error: Uncaught Error: strlen(): Argument #1 ($str) must be of type string, array given in /wp-content/plugins/admin-color-schemer/lib/phpsass/script/SassScriptFunction.php on line 207

Ideally, there's a new version of that PHPSass library that can be dropped in to replace the old one which accommodates modern PHP.

Otherwise, it appears those 3 errors can be patched while leaving the rest as-is to get things back into working order on PHP 8.0 from what I've tested:

  1. Replace those 2 sets of curly braces with square brackets
  2. Add if(gettype($this->args) == 'string'){ $this->args = array($this->args); } before $argc = count($this->args);
  3. Add $string = ''.$string; before $strlen = strlen($string);
    for the 3 respective files & errors above.

Additionally, it'd probably be good to update/add to the plugin's data in the main plugin file & readme info for mentioning the required PHP version (currently missing but 5.6 might be a fine option [haven't tested to be sure] unless 7.0 or something is a better option since those still on 5.6 are likely already needing to use old versions of plugins anyway & would be easier to test/guarantee), required WordPress version (3.8+ might be fine as-is), and the WordPress version it's been tested up to (currently missing while it can now be 6.2.2 to get rid of that warning on https://wordpress.org/plugins/admin-color-schemer/ that says the plugin hasn't been tested with modern WP versions) while also updating the plugin version, changelog, etc. accordingly.

*Keeping in mind that https://core.trac.wordpress.org/ticket/48881 might also still be happening for anyone with checkboxes that don't show up. I wonder if there's a good way where the checkbox just isn't given a custom style unless it's been specifically set in the custom admin color scheme setting for that one item to help avoid oddities regarding this.

I’ve also posted this at https://wordpress.org/support/topic/fatal-error-fix-php-8-x-fatal-error-upon-saving-custom-admin-color-scheme/ if it’s seen as beneficial to discuss/track things further there (and/or also putting this in one of the first spots one might look at when encountering this issue.)

@KZeni
Copy link
Contributor Author

KZeni commented May 30, 2023

Let me know if this set of patches to the PHPSass library included in this plugin should be submitted as a Pull Request as I'd be happy to do so (it really is those 3 individual file patches as described above [as well as the main PHP file & readme for the other info to be updated/added.) I did want to wait for input on this just in case there's a PHPSass fork or some equivalent that already has an updated version available that fixes this and would be seen as the preferable option to be done instead of the direct patching of those 3 errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant