Automattic / Automattic/nginx-http-concat
PHP Notices raised by cssconcat
- Dominant language
- PHP
- Stars
- 120
- Forks
- 37
- PR merge metrics
- No merged PRs in 30d
Description
When running unit tests for [Customize Widgets Plus](https://github.com/xwp/wp-customize-widgets-plus) inside of Quickstart, I'm noticing PHP Notices being raised:
```
Only variable references should be returned by reference
/srv/www/wp-content/mu-plugins/http-concat/cssconcat.php:174
/srv/www/wp/wp-includes/class.wp-dependencies.php:219
/srv/www/wp/wp-includes/script-loader.php:653
/srv/www/wp/wp-includes/plugin.php:579
/srv/www/wp/wp-includes/class.wp-styles.php:39
/srv/www/wp-content/mu-plugins/http-concat/cssconcat.php:31
/srv/www/wp-content/mu-plugins/http-concat/cssconcat.php:191
/srv/www/wp/wp-includes/plugin.php:503
/srv/www/wp-tests/tests/phpunit/tests/customize/widgets.php:53
/srv/www/wp-tests/tests/phpunit/tests/customize/widgets.php:175
```
The [code in question](https://github.com/Automattic/vip-quickstart/blob/a13fc5a809f536efa1b186b65689e62944180479/www/wp-content/mu-plugins/http-concat/cssconcat.php#L173-L175) is:
``` php
class WPcom_CSS_Concat extends WP_Styles {
// ...
function &__get( $key ) {
return $this->old_styles->$key;
}
// ...
}
```
I tried changing this method to:
``` php
function &__get( $key ) {
$value = $this->old_styles->$key;
return $value;
}
```
But then of course a different error is raised when `wp_styles()->registered[ $key ]` is attempted to be set:
```
Indirect modification of overloaded property WPcom_CSS_Concat::$registered has no effect
/srv/www/wp/wp-includes/class.wp-dependencies.php:221
/srv/www/wp/wp-includes/functions.wp-styles.php:116
/srv/www/wp-content/plugins/jetpack/_inc/genericons.php:11
/srv/www/wp/wp-includes/plugin.php:503
/srv/www/wp-tests/tests/phpunit/tests/customize/widgets.php:53
/srv/www/wp-tests/tests/phpunit/tests/customize/widgets.php:175
```
I think perhaps the approach for extending `WP_Styles` needs to be changed. Instead of storing the original instance of `WP_Styles` inside of the `WPcom_CSS_Concat` instance, I think it should be fully re-using all of its properties directly.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with www/wp-content/mu-plugins/http-concat/cssconcat.php around lines 173-175 and trace how WPcom_CSS_Concat proxies WP_Styles properties. Compare the failing paths in class.wp-dependencies.php and the customize/widgets.php tests. Done means the PHP notices and indirect-modification error no longer occur while the affected unit tests run.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100