WordPress.org

Make WordPress Themes

Opened 3 years ago

Closed 3 years ago

#22639 closed theme (live)

THEME: Adirondack - 0.1.3

Reported by: ryelle Owned by: iCaleb
Priority: new theme Keywords: theme-adirondack
Cc: fantastic.fall@…

Description

Adirondack - 0.1.2

Adirondack has a bright, clean layout designed to give your content the full attention it deserves. With large images and neat typography, this theme is crafted for longform writers and photographers alike.

Theme URL - http://themes.redradar.net/adirondack/documentation/
Author URL - http://themes.redradar.net

SVN - https://themes.svn.wordpress.org/adirondack/0.1.2
ZIP - https://wordpress.org/themes/download/adirondack.0.1.2.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/adirondack/0.1.1&new_path=/adirondack/0.1.2

History:

Ticket Summary Status Resolution Owner
#21730 THEME: Adirondack - 0.1.1 closed not-approved rabmalin
#22639 THEME: Adirondack - 0.1.3 closed live iCaleb

(this ticket)


https://themes.svn.wordpress.org/adirondack/0.1.2/screenshot.png

Change History (16)

#1 @themetracbot
3 years ago

  • Owner set to iCaleb
  • Status changed from new to reviewing

#2 @iCaleb
3 years ago

Well done on this theme! I really like the simplicity and there are some very interesting and unique approaches to the sidebar and comment system. I've reviewed over your theme, and cross-checked with the errors found in your previous ticket (https://themes.trac.wordpress.org/ticket/21730). There is a requirement that needs to be met before this theme may enter the theme repository.


Required

  • Use esc_attr() for text inputs and esc_textarea() for textareas. (This needs to be implemented in comments.php)

Notes

  • With the above items fixed, we should be able to get your theme pushed out into the repository in no time.

#3 @ryelle
3 years ago

Thanks for the review!

Where in comments.php are you seeing an issue?

I have my changelog in the readme & I've fixed the style issues, so once I hear back about the comments issue I'll upload the new version.

#4 @iCaleb
3 years ago

I believe that all the input/textareas need to be escaped. See this example from all of the default WP themes:

$fields =  array(

  'author' =>
    '<p class="comment-form-author"><label for="author">' . __( 'Name', 'domainreference' ) . '</label> ' .
    ( $req ? '<span class="required">*</span>' : '' ) .
    '<input id="author" name="author" type="text" value="' . esc_attr( $commenter['comment_author'] ) .
    '" size="30"' . $aria_req . ' /></p>',

  'email' =>
    '<p class="comment-form-email"><label for="email">' . __( 'Email', 'domainreference' ) . '</label> ' .
    ( $req ? '<span class="required">*</span>' : '' ) .
    '<input id="email" name="email" type="text" value="' . esc_attr(  $commenter['comment_author_email'] ) .
    '" size="30"' . $aria_req . ' /></p>',

  'url' =>
    '<p class="comment-form-url"><label for="url">' . __( 'Website', 'domainreference' ) . '</label>' .
    '<input id="url" name="url" type="text" value="' . esc_attr( $commenter['comment_author_url'] ) .
    '" size="30" /></p>',
);

#5 @ryelle
3 years ago

But I don't think I'm doing that - can you point out, in my comments.php, where you see the issue?

This ticket was mentioned in Slack in #themereview by icaleb. View the logs.


3 years ago

#7 @iCaleb
3 years ago

On line 72 in comments.php you are declaring a custom textarea. You need to use esc_textarea() for security.

https://make.wordpress.org/themes/handbook/review/required/#security-and-privacy

#8 @ryelle
3 years ago

Right, but there's nothing that needs escaping here. In the example you sent, the value attribute is being set to user-entered content, which is why it needs escaping. I don't have that in the textarea:

'<p class="comment-form-comment">
<label for="comment">' . _x( 'Comment', 'noun', 'adirondack' ) . '</label>
<div class="comment-wrap">
<textarea id="comment" name="comment" cols="45" rows="8" aria-required="true"></textarea>
</div></p>'

The only thing here is the translation, but I don't think I need to escape that-- it's not in the core example above, either.

#9 @iCaleb
3 years ago

Yes. You need to set the value and have it escaped I believe.

#10 @ryelle
3 years ago

There is no value- see the same line in core:

'<p class="comment-form-comment">
<label for="comment">' . _x( 'Comment', 'noun' ) . '</label>
<textarea id="comment" name="comment" cols="45" rows="8" aria-describedby="form-allowed-tags" aria-required="true" required="required"></textarea>
</p>'

I grabbed this and all I did to update it was add the wrapper div around the textarea.

#11 @ryelle
3 years ago

The $commenter data in the lines you listed comes from saved data from past comments - this way your name, email, and website are pre-filled when you make another comment. You wouldn't have the comment pre-filled though, so that should not have a set value.

#12 @themetracbot
3 years ago

  • Summary changed from THEME: Adirondack - 0.1.2 to THEME: Adirondack - 0.1.3

Adirondack - 0.1.3

Adirondack has a bright, clean layout designed to give your content the full attention it deserves. With large images and neat typography, this theme is crafted for longform writers and photographers alike.

Theme URL - http://themes.redradar.net/adirondack/documentation/
Author URL - http://themes.redradar.net

SVN - https://themes.svn.wordpress.org/adirondack/0.1.3
ZIP - https://wordpress.org/themes/download/adirondack.0.1.3.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=/adirondack/0.1.2&new_path=/adirondack/0.1.3

History:

Ticket Summary Status Resolution Owner
#21730 THEME: Adirondack - 0.1.1 closed not-approved rabmalin
#22639 THEME: Adirondack - 0.1.3 closed live iCaleb

(this ticket)


https://themes.svn.wordpress.org/adirondack/0.1.3/screenshot.png

#13 @ryelle
3 years ago

I've updated the theme with your suggested style changes. Let me know if you still think the comments are an issue.

#14 @karmatosed
3 years ago

@iCaleb are you able to continue the review?

#15 @iCaleb
3 years ago

  • Status changed from reviewing to approved

Oops, sorry @karmatosed! I lost track of this review in the process of starting my new job at WooThemes:)

And sorry @ryelle for the delayed reply! This theme looks to be all set according to the requirements, the only thing I am a little unclear about is the "esc_attr() for text inputs and esc_textarea() for textareas" issue.

I'm going to mark the ticket as approved based on you being correct, if the final reviewer sees an issue with esc thing, then we can get that squared away.

#16 @alex27
3 years ago

  • Priority changed from previously reviewed to new theme
  • Resolution set to live
  • Status changed from approved to closed

Everything looks good, marking the theme as LIVE.
You might want to switch to the_archive_title and the_archive_description() in archive template in future update.

Note: See TracTickets for help on using tickets.