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 for WordPress. #6

Open
wants to merge 1 commit into
base: master
from
Open

Fix for WordPress. #6

wants to merge 1 commit into from

Conversation

@kagg-design
Copy link

kagg-design commented Jan 7, 2020

In WordPress core, on the very early stage, the file wp-includes/compat.php is loaded. It defines polyfills for some mb_ functions, including mb_strlen. After this, Symfony\Polyfill\Mbstring becomes useless in any plugin / theme. This causes fatal errors on mb_ function calls.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 7, 2020

I'm not sure this solves the issue: https://github.com/WordPress/WordPress/blob/master/wp-includes/compat.php
When mb_strpos is not found, this will redeclare mb_strlen, thus fail, isn't it?

@kagg-design

This comment has been minimized.

Copy link
Author

kagg-design commented Jan 7, 2020

Yes, you are right, I was too fast. In my local install, I use

if ( ! extension_loaded('mbstring') ) {

and this works for me. I tried to make PR with minimal changes, but made is wrong. Why don't you use extension_loaded ? Should I fix PR with it?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 7, 2020

extension_loaded has the very same issue...
the best way is to do nothing here, but to load this file before wordpress loads its compat.php file

@kagg-design

This comment has been minimized.

Copy link
Author

kagg-design commented Jan 7, 2020

It is simply impossible without modification of WordPress core. This is an early stage of core load, and no plugins or theme are included at this moment.

@kagg-design

This comment has been minimized.

Copy link
Author

kagg-design commented Jan 7, 2020

Last fix is tested in WordPress plugin.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 7, 2020

thanks, works for me,
can you please fix the code style? we use 4 spaces for indentation

In WordPress core, on the very early stage, the file wp-includes/compat.php is loaded. It defines polyfills for some mb_ functions, including mb_strlen. After this, Symfony\Polyfill\Mbstring becomes useless in any plugin / theme. This causes fatal errors on mb_ function calls.
@kagg-design kagg-design force-pushed the kagg-design:master branch from bd1380e to 7726569 Jan 7, 2020
@kagg-design

This comment has been minimized.

Copy link
Author

kagg-design commented Jan 7, 2020

I hope did it right :) (my phpStorm is tuned for WordPress Coding Standards). Also, squashed 4 commits.

Could you tag this in case of acceptance? I will update our repositories.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.