Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#46313 closed defect (bug) (wontfix)

WordPress 5.1 and $themes backward compatibility break-ish

Reported by: flexithemes's profile 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 6 years ago.
46313.diff (762 bytes) - added by earnjam 6 years ago.

Download all attachments as: .zip

Change History (10)

@flexithemes
6 years ago

#1 @earnjam
6 years 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
6 years ago

#2 @earnjam
6 years ago

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

#3 @earnjam
6 years 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 6 years ago by earnjam (previous) (diff)

#4 @TimothyBlynJacobs
6 years ago

#46325 was marked as a duplicate.

#5 @AppleBag
6 years 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
6 years 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
6 years 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
6 years 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.