[theme-reviewers] help with review

[Chip Bennett]

Remember, though, textdomain MUST be a string value. This:

esc_attr_e( 'Search', WEBFISH_THEME_NAME . '-theme' );


Needs to be replaced with the actual string.

I'll make additional comments in the ticket, as well.

Chip

On Sat, Dec 24, 2011 at 5:43 AM, Michael Fields <michael at mfields.org> wrote:

> Hi Paul,
>
> > also, it is echoing a string without escaping it.
> > Author is arguing his code is ok and secure regardless.
> > what should I do?
>
> If the value is contained in an attribute, it should always be escaped if
> it contains a dynamic value. Translated strings are dynamic once
> translated. This can definitely cause issues with translated strings that
> include a single quote and the attributes value is enclosed in single
> quotes. The following is the best way to handle a situation such as this:
>
> <input name="s" type="text" id="s" value="<?php esc_attr_e( 'Search',
> WEBFISH_THEME_NAME . '-theme' ); ?>"/>
>
> Twenty Eleven provides a similar feature to what the theme author is going
> for here using the placeholder attribute. You may want to suggest that the
> author tries something like this instead:
>
> <input type="text" class="field" name="s" id="s" placeholder="<?php
> esc_attr_e( 'Search', 'twentyeleven' ); ?>" />
>
> My opinion is that the value of the search input be reserved for the
> search query. The term "Search" in the case is either a label or an action
> to be taken and should be reserved for the submit button, a label element
> or a placeholder attribute. Using it as the value of search input is bad
> form as id decreases usability of the form.
>
> > also, there is no margin below paragraphs, (readability test) -
> shouldn't that be required?
>
> I believe so. Paragraphs should be easily distinguished from one another.
>
> Best,
> -Mike
> _______________________________________________
> 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/20111224/cb5b890d/attachment.htm>