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

Potential loss of customizations when WP_INSTALLING=true #571

Closed
Nikeo opened this Issue Oct 3, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@Nikeo
Collaborator

Nikeo commented Oct 3, 2017

https://wordpress.org/support/topic/loss-of-customizations-when-viewing-wp-activate-php-in-as-admin/

Anytime we view wp-activate.php as an admin (happens when creating test users) we lose all the Customization settings for Hueman. I’ve tracked the issue down to WP_INSTALLING=true (see wp-activate.php) causing wp_cache_get( 'alloptions', 'options' ); to return false, instead of all options as expected. Because the admin user has the “edit_theme_options” permission hu_get_default_options() (hueman/functions/class-utils.php) decides it should rebuild the theme option cache with the incomplete options from wp_cache_get(). My temp solution is to edit hu_get_default_options() so that only when is_admin() == true and the user has the “edit_theme_options” permission do the theme options get rebuilt. This is a on a WP Network setup.

I don’t know if this is the optimal solution, but some solution should be implemented as I doubt I’m the only person that has noticed this odd behavior. A better solution may be checking if WP_INSTALLING=true and not updating the options, but I have a hard time believe that anytime an admin views a page the theme options should be rebuilt.

@Nikeo Nikeo added bug urgent labels Oct 3, 2017

@Nikeo Nikeo changed the title from Potential loss of customization when WP_INSTALLING=true to Potential loss of customizations when WP_INSTALLING=true Oct 3, 2017

@Nikeo Nikeo modified the milestones: 3.3.14, 3.3.21 Oct 3, 2017

@Nikeo Nikeo closed this in 4a3636a Oct 3, 2017

@eri-trabiccolo

This comment has been minimized.

Contributor

eri-trabiccolo commented Oct 3, 2017

[opinion]
Mmm, yeah I see. Some considerations:

  1. In Customizr, czr_fn_set_option doesn't call czr_fn_get_raw_option anymore but basically get_option (due to that issue with SG, do you remember?). I think that, as you said at that time, hu_set_option should not refer to hu_get_raw_option.
    What we wanted to avoid there was any 3rd party code (e.g. lang plugins) to filter the theme options making us save a "corrupted" theme option set.
    We can create something like hu_get_unfiltered_option which would be pretty much a copy of get_option but without the filters.
  2. what the user says makes anyway sense, when wp_installing() is true, we should not update the default options.
    https://github.com/presscustomizr/hueman/blob/v3.3.20/functions/class-utils.php#L246
    [/opinion]
@Nikeo

This comment has been minimized.

Collaborator

Nikeo commented Oct 3, 2017

[idad data-beverage="false"]
Yep.
I just pushed a first commit on hueman and checked that we were good for customizr
I'm also about to make sure we prevent updating the default options when WP_INSTALLING is true.
[/idad]

@Nikeo

This comment has been minimized.

Collaborator

Nikeo commented Oct 3, 2017

How is the tooth going ? ( maybe not the good place to have this type of chat, but hey, this is my repo after all ! :) )

@eri-trabiccolo

This comment has been minimized.

Contributor

eri-trabiccolo commented Oct 3, 2017

Ahaha, about idad :
yeah, good, wp 4.4.0 introduced wp_installing() function for that check (when no params are passed, otherwise sets ...)

The tooth, not very good, tomorrow morning I have to go to the dentist (maybe not the good place to have this type of chat, because hey, this is your repo not mine after all ! :- )

  • the emoticon :- is not a typo, it reflects the likely future of my teeth, just one, in the middle.
@Nikeo

This comment has been minimized.

Collaborator

Nikeo commented Oct 3, 2017

ah ok perfect, I'll use wp_installing() then.

ah ah, ok got it for your new smiling emoticon :))
Well, as long as you can bite with at least one teeth it's fine. And remember you have other nice features ( I know you are not a theme but I'm not sure about the word there ... ) like the beatutiful sk. for example :))

@eri-trabiccolo

This comment has been minimized.

Contributor

eri-trabiccolo commented Oct 3, 2017

Thanks for taking care of my self-esteem, I will talk about you to the beautiful sk. fraternity ;)

@ryanrolds

This comment has been minimized.

ryanrolds commented Oct 4, 2017

👍 Thanks for the quick response.

@Nikeo Nikeo added the fixed label Oct 6, 2017

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