WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#46313 closed defect (bug) (wontfix)

WordPress 5.1 and $themes backward compatibility break-ish

Reported by: flexithemes Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.1
Component: Themes Keywords:
Focuses: Cc:

Description

First of all, thank you for maintaining WordPress and making it awesome.

That said, WordPress 5.1 introduced what I consider a backward compatibility break, which has broken all 2,200+ of our WordPress themes.

The issue is with the $theme variable used in the themes. Seems that WordPress will use that variable in the core code from this version onward.

Therefore, this global variable produces null:

<?php
 global $theme;

And when reference through:

<?php
$theme->hook('meta');

The result is a fatal error message now, when things were working perfectly in 5.0 and previous.

Steps to replicate are simple:

  1. Install any of our themes on WordPress 5.1
  2. After activating, check the front end of the site.
  3. You'll see an error message like:

Fatal error: Call to a member function hook() on null in wp-content/themes/Teva/header.php on line 6

If you want to try that specific theme (i.e. Teva), I've attached it to this ticket.

Expected result: theme working as before

Server environment: PHP 7, LAMP ... I think it would repeatable on any server

Thoughts for discussion if that's allowed here... should the WordPress core namespace its variables?

Should theme developers be namespacing variables? Beyond this old guideline for namespacing, I haven't been able to find anything else official.

The conclusion there was:
"So, bottom line, TL:DR: forked/derivative stand-alone Themes ARE required to re-namespace; Themes that use framework libraries are NOT required to re-namespace the framework library code."

Final miscellaneous note: I set the severity to "critical". I wasn't able to find a definition in the glossary (or anywhere else) for what constitutes "critical", but since it's a fatal error so I decided to go with it.

Thank you for your time!

Attachments (2)

Teva.zip (694.7 KB) - added by flexithemes 5 months ago.
46313.diff (762 bytes) - added by earnjam 5 months ago.

Download all attachments as: .zip

Change History (10)

@flexithemes
5 months ago

#1 @earnjam
5 months ago

Looks like the changes to wp-settings.php in [44524] stepped on the $theme global.

<?php
// Load the functions for the active theme, for both parent and child theme if applicable.
foreach ( wp_get_active_and_valid_themes() as $theme ) {
        if ( file_exists( $theme . '/functions.php' ) ) {
                include $theme . '/functions.php';
        }
}
unset( $theme );

@earnjam
5 months ago

#2 @earnjam
5 months ago

46313.diff Just uses a different variable name as not to interfere with the $theme global.

#3 @earnjam
5 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Severity changed from critical to normal
  • Status changed from new to closed

Actually, now that I look into it, you should probably just be using some kind of variable prefix if you are going to use globals.

I don't think that global is actually used anywhere in core except in a very specific circumstance in /wp-includes/template-loader.php.

I'm going to close as wontfix, as core cannot account for every theme/plugin's use of vaguely named global variables.

Last edited 5 months ago by earnjam (previous) (diff)

#4 @TimothyBlynJacobs
5 months ago

#46325 was marked as a duplicate.

#5 @AppleBag
5 months ago

So if it's marked as wontfix, what's the correct fix (example please) for older themes that use global $theme; that won't require core hacking?

#6 @flexithemes
5 months ago

@AppleBag I can't speak for other theme companies, but in our situation we created a small script that renamed the variable.

<?php

        function create_edit_file($file, $content) {
          $fp = fopen($file, 'w');
          fwrite($fp, $content);
          fclose($fp);

          return true;
        }

        function replaceInExact($file, $searchfor, $replacewith) {
          $get_file = file_get_contents($file);
          if ($save_content = str_replace($searchfor, $replacewith, $get_file)) {
                create_edit_file($file, $save_content);
          }

          return true;
        }

        function wp51update_files($folder) {
                if ($handle = opendir($folder)) {
                        while (false !== ($file = readdir($handle))) {
                                if ($file != '.' && $file != '..' && $file != 'Thumbs.db'  ) {
                                  if (is_dir($folder . '/' . $file ) ) {
                                        wp51update_files( $folder . '/' . $file );
                                  } elseif( substr_count($file, '.php') > 0 )  {
                                        replaceInExact($folder . '/' . $file, '$theme', '$flexitheme');
                                  }
                                }
                        }
                        closedir($handle);
                }
        }
  
        $folder = './';

        wp51update_files($folder);

        echo "Sweet! Everything updated fine and your site should be working now!";

#7 @earnjam
5 months ago

@flexithemes You really should not modify core files like that. Pretty bad idea for a number of reasons.

All you need to do is just change the name of the global variable you are using in the theme. You could add some sort of prefix (example: $teva_theme).

But the best solution would simply be to not use a global variable at all. This is a good example of why they are problematic and generally should be avoided if possible.

#8 @earnjam
5 months ago

@flexithemes, sorry, I misread the code. I see now that you mean to execute that inside your theme directory.

Most code editors/IDEs will also allow you to search-replace across a full directory or list of files.

Note: See TracTickets for help on using tickets.