WordPress.org

Ready to get started?Download WordPress

Review WordPress Themes

Updates from Samuel Wood (Otto) Toggle Comment Threads | Keyboard Shortcuts

  • Samuel Wood (Otto) 5:59 pm on May 23, 2012 Permalink | Log in to leave a Comment  

    Not sure how to codify this into a rule or something, but putting it out there for thought and discussion.

    Themes should not “do” anything merely from their files being loaded. I know there’s been ways to have themes do something on “activation”, but in general, this is bad.

    This gets really, really bad with 3.4 and the theme customizer. See, a theme has no idea if it’s actually live or just being displayed in the customizer. So if a theme does something like changing the blog settings or some such thing just because it’s loaded, then this could cause weird side effects when it’s not actually the active theme. Nobody wants to preview a theme and have their main blog page suddenly change.

    This “don’t do anything by default” includes changing anything in the database whatsoever, writing files, making posts, etc. All actions taken by a theme must in some way actually be user-initiated.

    Discuss.

     
    • Daniel 6:16 pm on May 23, 2012 Permalink | Log in to Reply

      I believe an important aspect of this is to know there are other ways of setting the theme up than saving in the database. For example theme options can be passed a default value as a second parameter (e.g. get_options( ‘theme_options’, $default_theme_options ) ), mods like background images, header images and colors can also have default values passed. I believe these implementations should be pushed forward as strongly recommended best-practices.

    • Edward Caissie 6:22 pm on May 23, 2012 Permalink | Log in to Reply

      This “don’t do anything by default” includes changing anything in the database whatsoever, writing files, making posts, etc. All actions taken by a theme must in some way actually be user-initiated.

      I’m not so sure this really needs much discussion … it should simply be the expected standard.

      • Otto 6:27 pm on May 23, 2012 Permalink | Log in to Reply

        Even WP’s own themes are guilty of this to some degree (we’re talking about P2 in IRC), so it needs to be discussed a bit at least.

        I’m more thinking along the lines of how to best communicate this to theme authors. I mean, I could write some tut posts about the right way to use defaults and such, if it helps. It’s not something that we can check for with theme check easily, and the concept is a bit high-level anyway.

        We’re also talking about changing core to try to mitigate this in the case of the previewer, by simply blocking add_option and similar calls when the previewer is active. Not sure what side effects this could have though.

        • Thomas Scholz 11:33 pm on May 23, 2012 Permalink | Log in to Reply

          Given the mass of themes that aren’t updated in the near future … it would be useful to catch those cases in the theme preview, I think.

          As a role it is good and reasonable – but not safe enough. Blocking add_option() should work.

        • scribu 1:09 am on May 24, 2012 Permalink | Log in to Reply

          If we can pull off an automated solution, we should.

          Themes that don’t make any provision for when their activation code was run will break when loaded through the previewer, which is a good thing, IMO, since it will force the authors to make them more resilient.

    • Jeff Sebring 1:23 am on May 24, 2012 Permalink | Log in to Reply

      Would you classify setting default theme mods as one of these cases?

      I don’t see the harm in that, since each theme has it’s own row in the options table. If a theme is using mods for different display options, it would need to have these values set to preview.

    • figerty 4:19 am on May 29, 2012 Permalink | Log in to Reply

      I’m with Jeff. If a theme has a particular modification that is required by the theme to function correctly or do something specific to the theme then it should be allowed. But I agree with Otto too. There is nothing worse when installing a theme, whther for review or just to see what it’s like, and the theme installing other components or text/images etc that were not expected and having to manually remove them again. It’s a pain in the butt-end and should not be allowed.

    • shawn 1:24 am on June 14, 2012 Permalink | Log in to Reply

      Sorry I’m late what happened to this issue now that 3.4 is out, any solutions or do we proceed as per usual???

      • Chip Bennett 12:33 pm on June 14, 2012 Permalink | Log in to Reply

        Consider this an “operating philosophy”, unless/until we put specific verbiage into the Guidelines. If you have a specific use-case that you need help with, just email the theme-reviewers mail-list, so we can help you.

    • bitoolean 9:48 am on June 14, 2012 Permalink | Log in to Reply

      I haven’t tested WordPress v3.4, so I’m not sure which parts of a page would be run/rendered in a live preview (shortcodes, sharing widgets etc.), but it is obvious that incompatible/old themes may lead to a disaster by being previewed, so why not make it a requirement for any previewable page to specify that it is compatible (maybe even the wordpress version), for example in an HTML comment, and check that before previewing.

      • Chip Bennett 12:38 pm on June 14, 2012 Permalink | Log in to Reply

        I don’t think any of this actually changes in WordPress 3.4. The issue that Otto is bringing up in this post is Themes that do things upon activation – like changing core settings, inserting posts, etc.

  • Samuel Wood (Otto) 8:06 pm on February 11, 2011 Permalink
    Tags:   

    I’ve made a few changes to the theme uploader tool. There shouldn’t be any visible changes, but email me if anything looks weird on the next few theme tickets. Thanks!

    (It’s mostly just prep work for proper child theme support, nothing of special interest.)

     
c
compose new post
j
next post/next comment
k
previous post/previous comment
r
reply
e
edit
o
show/hide comments
t
go to top
l
go to login
h
show/hide help
shift + esc
cancel