<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[55580] trunk: Code Modernization: Fix dynamic properties in WP_Admin_Bar.</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { white-space: pre-line; overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta" style="font-size: 105%">
<dt style="float: left; width: 6em; font-weight: bold">Revision</dt> <dd><a style="font-weight: bold" href="https://core.trac.wordpress.org/changeset/55580">55580</a><script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","description":"Review this Commit","action":{"@type":"ViewAction","url":"https://core.trac.wordpress.org/changeset/55580","name":"Review Commit"}}</script></dd>
<dt style="float: left; width: 6em; font-weight: bold">Author</dt> <dd>hellofromTonya</dd>
<dt style="float: left; width: 6em; font-weight: bold">Date</dt> <dd>2023-03-21 19:58:45 +0000 (Tue, 21 Mar 2023)</dd>
</dl>
<pre style='padding-left: 1em; margin: 2em 0; border-left: 2px solid #ccc; line-height: 1.25; font-size: 105%; font-family: sans-serif'>Code Modernization: Fix dynamic properties in WP_Admin_Bar.
To fix the dynamic properties, the following changes are included:
* Removes `WP_Admin_Bar::__get()`.
* Declares `menu` as a property on the class, deprecates it, and initializes it to an empty array.
* Removes the unused 'proto' dynamic property.
Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.
== Why remove the `WP_Admin_Bar::__get()` magic method?
tl;dr
The magic method is no longer needed.
The magic method only handled the `menu` and `proto` dynamic properties. Introducing a full set of magic methods is overkill for this class. Instead of having to maintain magic methods, this changeset instead directly addresses the 2 properties (see below).
== Why declare the `menu` property on the class?
tl;dr
To simplify the code while maintaining backwards compatibility for extenders who are using this deprecated property.
The `menu` property was introduced during the 3.1.0 ''development cycle'' as a declared property <a href="https://core.trac.wordpress.org/changeset/15671">[15671]</a>. Its purpose was to ''internally'' hold the menu structure.
During the WP 3.3.0 development cycle, it was replaced by a new `private` property called `nodes` (see <a href="https://core.trac.wordpress.org/changeset/19120">[19120]</a>).
But breakage reports from extenders caused it to be restored. <a href="https://core.trac.wordpress.org/changeset/19501">[19501]</a> added the `__get()` magic method, i.e. for handling it as a dynamic property, and deprecated it.
>We're not going to maintain compat for $menu. Suggest we make it array() and plugins will have to deal. We can throw a _deprecated_argument() and push them to use the new methods.
~ Source: [https://core.trac.wordpress.org/ticket/19371#comment:17 see <a href="https://core.trac.wordpress.org/ticket/19371">#19371</a> comment 17]
[https://wpdirectory.net/search/01GSTW1X69TBN8FH3SY7V8KPY5 A search of the wp.org plugins and themes repository] shows that a few plugins are still using this deprecated property. To maintain backwards compatibility, `menu` is moved back to the class as a declared property, set to an empty array (as it's been since 3.3.0), and deprecated in the property's description.
== Why remove the `proto` dynamic property?
tl;dr
* It was not intended to be released in 3.1.
* There are no usages of it in Core or in the WP.org's plugin or theme directories.
* It should be safe to remove.
This property was first introduced in the WP 3.1.0 ''development cycle'' to replace the `PROTO` constant (see <a href="https://core.trac.wordpress.org/changeset/16038">[16038]</a>) for protocol handling for the admin bar's hyperlinks. <a href="https://core.trac.wordpress.org/changeset/16077">[16077]</a> replaced the property's usages with URL functions such as `get_admin_url()` and `admin_url()`. But it missed removing the property, which was no longer needed or used.
It was relocated to the `__get()` magic method as a dynamic property when the `menu` property was restored (see <a href="https://core.trac.wordpress.org/changeset/19501">[19501]</a>).
A search of WP.org's plugins and themes repositories shows no usages of the property. Core hasn't used it since the removed in <a href="https://core.trac.wordpress.org/changeset/16038">[16038]</a> before 3.1 final release. It should be safe to remove it, but committing very early in the 6.3 alpha cycle to give time for reports of usages, if there are any.
References:
* A [https://www.youtube.com/watch?v=vDZWepDQQVE&t=9362s live open public working session] where these changes were discussed and agreed to.
* [https://wiki.php.net/rfc/deprecate_dynamic_properties PHP RFC: Deprecate dynamic properties].
Follow-up to <a href="https://core.trac.wordpress.org/changeset/19501">[19501]</a>, <a href="https://core.trac.wordpress.org/changeset/19120">[19120]</a>, <a href="https://core.trac.wordpress.org/changeset/16308">[16308]</a>, <a href="https://core.trac.wordpress.org/changeset/16038">[16038]</a>, <a href="https://core.trac.wordpress.org/changeset/15671">[15671]</a>.
Props antonvlasenko, hellofromTonya, jrf, markjaquith, desrosj, ironprogrammer, peterwilsoncc, SergeyBiryukov.
See <a href="https://core.trac.wordpress.org/ticket/56876">#56876</a>, <a href="https://core.trac.wordpress.org/ticket/56034">#56034</a>.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunksrcwpincludesclasswpadminbarphp">trunk/src/wp-includes/class-wp-admin-bar.php</a></li>
<li><a href="#trunktestsphpunittestsadminbarphp">trunk/tests/phpunit/tests/adminbar.php</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunksrcwpincludesclasswpadminbarphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/src/wp-includes/class-wp-admin-bar.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/src/wp-includes/class-wp-admin-bar.php 2023-03-21 17:37:17 UTC (rev 55579)
+++ trunk/src/wp-includes/class-wp-admin-bar.php 2023-03-21 19:58:45 UTC (rev 55580)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -19,22 +19,15 @@
</span><span class="cx" style="display: block; padding: 0 10px"> public $user;
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * @since 3.3.0
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * Deprecated menu property.
</ins><span class="cx" style="display: block; padding: 0 10px"> *
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * @param string $name
- * @return string|array|void
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @since 3.1.0
+ * @deprecated 3.3.0 Modify admin bar nodes with WP_Admin_Bar::get_node(),
+ * WP_Admin_Bar::add_node(), and WP_Admin_Bar::remove_node().
+ * @var array
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- public function __get( $name ) {
- switch ( $name ) {
- case 'proto':
- return is_ssl() ? 'https://' : 'http://';
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ public $menu = array();
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- case 'menu':
- _deprecated_argument( 'WP_Admin_Bar', '3.3.0', 'Modify admin bar nodes with WP_Admin_Bar::get_node(), WP_Admin_Bar::add_node(), and WP_Admin_Bar::remove_node(), not the <code>menu</code> property.' );
- return array(); // Sorry, folks.
- }
- }
-
</del><span class="cx" style="display: block; padding: 0 10px"> /**
</span><span class="cx" style="display: block; padding: 0 10px"> * Initializes the admin bar.
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span></span></pre></div>
<a id="trunktestsphpunittestsadminbarphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/tests/phpunit/tests/adminbar.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/tests/phpunit/tests/adminbar.php 2023-03-21 17:37:17 UTC (rev 55579)
+++ trunk/tests/phpunit/tests/adminbar.php 2023-03-21 19:58:45 UTC (rev 55580)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -754,4 +754,33 @@
</span><span class="cx" style="display: block; padding: 0 10px"> 'network-admin-o' => 'manage_network_options',
</span><span class="cx" style="display: block; padding: 0 10px"> );
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+
+ /**
+ * This test ensures that WP_Admin_Bar::$proto is not defined (including magic methods).
+ *
+ * @ticket 56876
+ * @coversNothing
+ */
+ public function test_proto_property_is_not_defined() {
+ $admin_bar = new WP_Admin_Bar();
+ $this->assertFalse( property_exists( $admin_bar, 'proto' ), 'WP_Admin_Bar::$proto should not be defined.' );
+ $this->assertFalse( isset( $admin_bar->proto ), 'WP_Admin_Bar::$proto should not be defined.' );
+ }
+
+ /**
+ * This test ensures that WP_Admin_Bar::$menu is declared as a "regular" class property.
+ *
+ * @ticket 56876
+ * @coversNothing
+ */
+ public function test_menu_property_is_defined() {
+ $admin_bar = new WP_Admin_Bar();
+ $this->assertTrue( property_exists( $admin_bar, 'menu' ), 'WP_Admin_Bar::$proto property should be defined.' );
+
+ $menu_property = new ReflectionProperty( WP_Admin_Bar::class, 'menu' );
+ $this->assertTrue( $menu_property->isPublic(), 'WP_Admin_Bar::$menu should be public.' );
+
+ $this->assertTrue( isset( $admin_bar->menu ), 'WP_Admin_Bar::$menu should be set.' );
+ $this->assertSame( array(), $admin_bar->menu, 'WP_Admin_Bar::$menu should be equal to an empty array.' );
+ }
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span></span></pre>
</div>
</div>
</body>
</html>