Five things we learned from getting our theme reviewed
By 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:
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.
