[theme-reviewers] Fwd: [WordPress Themes] #1596: THEME: Simply Works Core - 1.3.3

Wed Oct 20 15:08:32 UTC 2010

I would leave the verbiage to the "best practices" sections that we will
hopefully soon have time to start adding to the Codex.

JMO, of course.

Chip

On Wed, Oct 20, 2010 at 10:02 AM, Edward Caissie
<edward.caissie at gmail.com>wrote:

> Yes, I would agree with that.
>
> The PHP mention is just an idea to consider with the additional verbage
> that is part of the new draft for license declaration requirements.
>
>
> Cais
>
>
> On Wed, Oct 20, 2010 at 10:52 AM, Chip Bennett <chip at chipbennett.net>wrote:
>
>> I think that, unless stated otherwise, that the *License* and *License
>> URI* header tags cover the Theme as a whole.
>>
>> Chip
>>
>>
>> On Wed, Oct 20, 2010 at 9:44 AM, Edward Caissie <edward.caissie at gmail.com
>> > wrote:
>>
>>> A lot of follow-up reading to be done there, and some learning curves to
>>> be straightened out, too.
>>>
>>> Interesting last note about the PHP being released under the GPL, that
>>> should be added to the license declarations ... especially for the WP3.1
>>> Theme Review guidelines.
>>>
>>>
>>> Cais.
>>>
>>> On Wed, Oct 20, 2010 at 10:06 AM, Chip Bennett <chip at chipbennett.net>wrote:
>>>
>>>> All,
>>>>
>>>> Please see Nacin's security review comments below.
>>>>
>>>> Perhaps a good starting point for developing some security-related
>>>> Guidelines? (Note: this is mostly above my pay grade. I'll be in
>>>> watch-and-learn mode. But I definitely think we need some Theme-Review
>>>> guidance wrt security.)
>>>>
>>>> And, thanks to Nacin for taking the time to perform the security review!
>>>>
>>>> Chip
>>>>
>>>> ---------- Forwarded message ----------
>>>> From: WordPress Themes <theme-reviewers at lists.wordpress.org>
>>>> Date: Wed, Oct 20, 2010 at 8:53 AM
>>>> Subject: Re: [WordPress Themes] #1596: THEME: Simply Works Core - 1.3.3
>>>> To:
>>>>
>>>>
>>>> #1596: THEME: Simply Works Core - 1.3.3
>>>>
>>>> --------------------------+-------------------------------------------------
>>>>  Reporter:  simplyworks  |      Owner:  nacin
>>>>      Type:  theme        |     Status:  assigned
>>>> Resolution:               |   Keywords:  theme-simply-works-core,
>>>>
>>>> --------------------------+-------------------------------------------------
>>>>
>>>> Comment(by nacin):
>>>>
>>>>  In terms of security: Better than most, but still not great. Please
>>>> read
>>>>  http://codex.wordpress.org/Data_Validation.
>>>>
>>>>  - User input into text options fields are not being sanitized. Seeing a
>>>>  lot of REQUEST on the same line as update_option. That's no good.
>>>>
>>>>  - check_admin_referer() isn't enough. You need current_user_can checks
>>>>  there too.
>>>>
>>>>  - Seeing a lot of form fields whose data/values/names etc isn't being
>>>>  escaped properly.
>>>>
>>>>  - bloginfo('name'); can't be used in the RSS feed title or any
>>>> attribute,
>>>>  as there are a few instances of it, without getting esc_attr().
>>>> Otherwise
>>>>  it may escape out of the attribute.
>>>>
>>>>  - get_stylesheet_directory() exposes the path. You don't want that. You
>>>>  want the URL.
>>>>
>>>>  Non-security notes:
>>>>
>>>>  - Custom header should be used instead of swc_logo.
>>>>
>>>>  - in index.php, you have an is_attachment() inside of an is_single().
>>>>  That will never be true. single, page, and attachment are all mutually
>>>>  exclusive. They do however all mean is_singular() == true.
>>>>
>>>>  - comment.php doesn't need the SCRIPT_FILENAME stuff.
>>>>
>>>>  - Not a fan of the direct query in list_most_commented(), or the non-
>>>>  prefixed custom functions -- register_my_menus() isn't good enough IMO.
>>>>
>>>>  - In addition to the non-prefixed functions, there's for example a
>>>> global
>>>>  called "$options" in dashboard.php. That needs to be prefixed, or
>>>>  preferably even not just sitting there as a global. Same with themename
>>>>  and shortname -- avoid globals where possible.
>>>>
>>>>  - Some strings aren't translated. Many are also not translatable, and
>>>> you
>>>>  should instead use sprintf syntax instead of having dangling strings
>>>> such
>>>>  as "By: ". Also, avoid escaping in translated strings, it annoys
>>>>  translators. Also, some strings such as "ADD COMMENT" should be "Add
>>>>  Comment", then text-transform:uppercase via CSS. This is better for all
>>>>  involved, including accessibility reasons.
>>>>
>>>>  - Don't delete options that store yes/no values. You're causing
>>>>  unnecessary queries as then those values are not autoloaded. Instead,
>>>>  store the literal value.
>>>>
>>>>  - add_theme_page() should not use `__FILE__`. Be more descriptive.
>>>> Also,
>>>>  edit_themes should be edit_theme_options. (Or switch_themes in
>>>> pre-3.0.)
>>>>  edit_themes controls *ONLY* the theme editor and may very well be
>>>> denied
>>>>  to everyone in many environments, like multisite.
>>>>
>>>>  - "The CSS, XHTML and design is released under GPL" -- and the PHP?
>>>>
>>>> --
>>>> Ticket URL: <http://themes.trac.wordpress.org/ticket/1596#comment:3>
>>>> WordPress Themes <http://themes.trac.wordpress.org/>
>>>> WordPress.org Theme Directory Reviews
>>>>
>>>>
>>>> _______________________________________________
>>>> theme-reviewers mailing list
>>>>
>>>> theme-reviewers at lists.wordpress.org
>>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>>
>>>>
>>>
>>> _______________________________________________
>>> theme-reviewers mailing list
>>> theme-reviewers at lists.wordpress.org
>>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>>
>>>
>>
>> _______________________________________________
>> theme-reviewers mailing list
>> theme-reviewers at lists.wordpress.org
>> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>>
>>
>
> _______________________________________________
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
> http://lists.wordpress.org/mailman/listinfo/theme-reviewers
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wordpress.org/pipermail/theme-reviewers/attachments/20101020/f59d100e/attachment-0001.htm>