[theme-reviewers] help with review
chip at chipbennett.net
Sat Dec 24 13:16:10 UTC 2011
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.
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.
> theme-reviewers mailing list
> theme-reviewers at lists.wordpress.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the theme-reviewers