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

Missing option class for wpseo-gsc option #11132

Open
2 tasks done
felixarntz opened this issue Oct 1, 2018 · 1 comment
Open
2 tasks done

Missing option class for wpseo-gsc option #11132

felixarntz opened this issue Oct 1, 2018 · 1 comment

Comments

@felixarntz
Copy link
Contributor

  • I've read and understood the contribution guidelines.
  • I've searched for any related issues and avoided creating a duplicate issue.

Please give us a description of what happened.

In Yoast_Form, the $options property can unexpectedly have a value of null instead of the intended array of option keys and their values.

Details

While the bug reported in #11126 can be fixed by adding a few extra conditional checks, there appears to be a more foundational issue originally causing it, which is that Yoast SEO (as well as the addons) partly access options that don't have a respective WPSEO_Option-derived class accompanying them. When such options are used with Yoast_Form to handle admin UI, the WPSEO_Options::get_option( ... ) call returns null because it only considers options that have an accompanying class.

The wpseo-gsc option is just an example case where this issue occurs, there might be others (possibly in addons), for example in wpseo-news as indicated in #11126.

Please describe what you expected to happen and why.

The Yoast_Form::$options property should hold an array of option keys and their values, as is expected and as the docblock states.

How can we reproduce this behavior?

  1. Add a var_dump( $this->options ) at the end of Yoast_Form::set_option().
  2. Access the Search Console page.
  3. See that the value outputted is null instead of an array.

Suggestion to fix

We could either implement option classes for all the missing options, which seems to be the clean approach to me. Addons could "register" their option classes by expanding the WPSEO_Options::$options property.

Alternatively, additional Yoast SEO options could be added to WPSEO_Options through some whitelist so that at least calls such as WPSEO_Options::get_option() work correctly. This would not require us to write classes for all the options, but might thus introduce other unexpected issues in the future.

@felixarntz
Copy link
Contributor Author

During work on #11126 I noticed that the Yoast_Form::$options property no longer retrieving a non-whitelisted option was introduced in 3841d9d. This is fixed through #11133 as well.

Regardless, this issue of not having dedicated option classes remains valid.

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

No branches or pull requests

3 participants