SAVE 25% when you purchase our Theme Bundle and Make Plus together. View bundle deal.

Five things we learned from getting our theme reviewed

By Corey McKrill on October 1, 2015

A few months ago, we asked Justin Tadlock and Emil Uzelac, of Theme Review, Co. to undertake a code review of Make and Make Plus. For those who don’t know, both are senior reviewers at WordPress.org (and Emil is now a reviewer at Envato). It was humbling — and also a great experience to have these two well-respected theme experts in the WordPress community take a deep dive into our code and scrutinize every line.

Now that we’ve had some time to collect our thoughts on the process, here are some of the things we learned from Justin and Emil’s feedback, that we think will help other theme developers better their own code.

Translated strings should be escaped too

Justin and Emil noted that we had done an excellent job escaping output from the database and user inputs. This is a standard practice for keeping code secure and preventing XSS vulnerabilities. However, one area that we had overlooked in some cases was translatable strings. Any string that’s included inside one of the translation functions, such as

__( ‘Hello world.’, ‘make’ )

will be replaced with the corresponding string in an .mo file, if it exists. Each language has its own .mo file, usually provided by a different translator. Theoretically, malformed or malicious code could be included in an .mo file and get inserted into a page load when the default string is swapped out. The way to prevent this from being a security issue is to use the escaped version of each translation function. So then

__( ‘Hello world.’, ‘make’ )

becomes

esc_html__( ‘Hello world.’, ‘make’ )

We have always reviewed each of the translations that we ship with Make to ensure that they didn’t contain anything malformed or malicious. But since we can’t control custom translation files or translations provided through the WordPress.org theme translation project, escaping all of our translation strings provides an extra layer of security hardening.

Good translator notes make for better translations

Speaking of translations, our theme had several instances where a translatable string, taken out of context, was practically meaningless to anyone trying to translate it using the theme’s .pot file. Justin and Emil helped us identify several of these instances and pointed out that we could provide context in a code comment near the translation string, and it would be included in the .pot file. Example:

Did you know that Surf Office, Postmatic, Yeah Dave, and over 700,000 small businesses run their websites with Make, our free WordPress page builder. Discover the Make page builder now.

Old .pot file entry
msgid "f/"
msgstr ""

New .pot file entry
#. Translators: this string denotes a camera f-stop. %s is a placeholder for
#. the f-stop value. E.g. f/3.5
msgid "f/%s"
msgstr ""

Some things actually shouldn’t be prefixed

One of the first things that’s drilled into your head as a WordPress developer is that everything should be prefixed so that your code will avoid collisions with other plugins’ code as well as core WordPress code. So in Make, all of our functions look like this:

ttfmake_function_name()

Many of our CSS classes look like this:

.ttfmake-class-name {}

One exception to this best practice is when it comes to naming 3rd party libraries. In Make, we include the Font Awesome icon font library. So do many plugins. If we all prefix our libraries with something like “ttfmake-font-awesome”, users may end up loading multiple instances of Font Awesome on a single page. If, however, everyone uses the same name, “font-awesome”, we can ensure that WordPress will only load the library once.

Clean, uncluttered templates make life easier for child themers, and for us

Justin and Emil noted that in several of our template files, we have a lot of logic to execute before we even get to the HTML. If you need to override one of these templates in a child theme, having all that logic in there adds a lot of complication, as well as increasing the risk that a future update of the parent theme will cause the child theme to become incompatible. For us, this increases the possibility that we will have a lot of redundant code spread throughout multiple template files.

The solution is to move much of that logic into functions elsewhere, and simply use those functions to bring only the pieces of data you need into the template files themselves. This is an area that we’re still working to improve.

Getting a third-party code review is totally worth it

If you’re a theme or plugin developer, and you’re on the fence about having your code reviewed, either before you release it into the wild or even for an established product, we’d encourage you to go for it. Reacting to user bug reports and feature requests will only get you so far in terms of improving your codebase. Getting objective feedback from a trusted third party will not only help you make your product better, but it will help you learn and grow as a developer — both of which are worthy investments. We take pride in the quality of our code here at The Theme Foundry, and getting third-party feedback on it has been a valuable part of making it even better.

So how did Make fare?

In Justin’s words:

I’ve reviewed 100s of WordPress themes and plugins over the years. I’ve seen the good, the bad, and the ugly. However, I can count the number of times on one hand when I’ve seen works of art. If code is truly poetry, then The Theme Foundry is the Shakespeare, Dickinson, and Whitman of our time, all rolled into one. Make and Make Plus represent some of the finest work I’ve seen.

Download Make for free and tell us what you think.

Enjoy this post? Read more like it in From the workshop.

6 Comments

  1. Jeffrey Donenfeld

    Awesome! Glad to see your initiative, relatively minor updates needed, and glowing summary from Justin. Can’t wait to install the next Make Plus update with these (small) issues taken care of. :)

  2. Corey McKrill

    Thanks Jeffrey! Actually, the latest versions of Make and Make Plus already have most of these improvements, although “declutter the template files” is an ongoing process.

  3. Matt Cromwell

    Great work! Really awesome endorsement from Justin for you guys. Have to say I agree completely!

    Is there any official documentation suggesting that front-end translateable strings must be escaped? Or that malicious code can be output via a .mo file? That’s new to me and I think that could affect TONS of themes and plugins. Any resource you have on that would be useful. Thanks!

  4. Corey McKrill

    Hey Matt!

    I haven’t found any specific case studies of bad things getting into .mo files. However, the Underscores starter theme now escapes all of its translations (example) and WordPress.com now requires it from all themers when submitting new themes.

    Generally, if you are shipping .mo files with your theme/plugin, you can ensure that there’s nothing bad in them. However, now that translations are available for themes and plugins on WordPress.org through language packs, you, as a developer, don’t have as much control over what’s in those translations anymore (unless you opt out of the feature).

    Which isn’t to say language packs are somehow bad. It just means that escaping translation strings is going to make your theme/plugin more secure.

  5. Daniel Fascia

    I’m sure that the adoption of ‘the Tadlock method for child theme CSS loading’ that I nudged you on a while ago helped the best practices review process too :-)

    But seriously, the fact such minor things were pointed out is a testament to how great Make and Make plus are. That’s why I use them as a base framework for every project at the moment.

    Well done, and keep up the work with the carefully considered incremental updates you do.

Comments are closed.