Mike -<br><br>I would say for the most part yes; this is part of the "eyes-on" approach of the review process though.<br><br>There may be cases when it is simply within the Theme design to not have "headings" properly cleared within a post's content but I cannot think of any that would be correct in the use of an "h" tag.<br>
<br><br>Cais.<br><br><div class="gmail_quote">On Mon, Mar 28, 2011 at 10:10 AM, <span dir="ltr"><<a href="mailto:michael@mfields.org">michael@mfields.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Cais,<br>
<br>
Thanks for taking the time to answer my questions! I'm still a bit foggy<br>
on #3 though. Are headings within post_content intended to be clearing<br>
blocks? I know that Twentyten does this, but honestly, this was the first<br>
time I had ever seen it done before. It's a good idea IMO and I do it<br>
myself now. Is it expected that all themes do this too?<br>
<br>
Best,<br>
-Mike<br>
<div><div></div><div class="h5"><br>
> Hey Mike -<br>
><br>
> re: Questions:<br>
><br>
> 1. I'm not really having issues with this considering you stated it is<br>
> being<br>
> escaped correctly.<br>
><br>
> 2. PHP notices are generally enough to set to "not-approved" as we expect<br>
> there to be none in the theme.<br>
><br>
> 3. Clearing tags appropriately is also generally considered for<br>
> "not-approved" but not usually if it is a single issue and no others are<br>
> found.<br>
><br>
> 4. Given that not passing the Theme Check will actually not allow a theme<br>
> to<br>
> be uploaded now you could stop there; but, since the theme's currently in<br>
> the queue were not subjected to that criteria they can be further<br>
> reviewed.<br>
> Of course, if you considered the Theme Check review to be epically failed<br>
> then there really is not much sense as I see it in continuing.<br>
><br>
><br>
> The Theme Review comments appear well written and informative ... without<br>
> actually reviewing the theme myself I would say you have given it a proper<br>
> review.<br>
><br>
><br>
> Cais.<br>
><br>
> On Sat, Mar 26, 2011 at 10:48 PM, <<a href="mailto:michael@mfields.org">michael@mfields.org</a>> wrote:<br>
><br>
>> Thanks Chip!<br>
>><br>
>> Gave the first theme a look over and I wanted to get some feedback from<br>
>> everyone about my observations. This is the first theme that I have<br>
>> reviewed so I am not sure about a few things.<br>
>><br>
>> It appears that this theme has undergone a cursory review by Chip<br>
>> Bennett<br>
>> <a href="http://themes.trac.wordpress.org/ticket/3015" target="_blank">http://themes.trac.wordpress.org/ticket/3015</a> In this scenario, I'm<br>
>> guessing it would be up to me to test all other areas not covered in the<br>
>> previous review. I've checked quite a few things at this stage and<br>
>> personally I would fail the theme. I've listed the following reasons why<br>
>> below. Please let me know your thoughts on what is causatioon for a<br>
>> failed<br>
>> review - I'm still a bit confused here. I also have a few questions if<br>
>> someone has the time.<br>
>><br>
>> Thanks!<br>
>> -Mike<br>
>><br>
>> Questions:<br>
>><br>
>> 1. The theme provides a textarea in a custom settings page that allows<br>
>> the<br>
>> user to display javascript in the frontend. My tests so far have shown<br>
>> that care has been taking so that the code is correctly escaped on the<br>
>> backend however the code is executed in the template files. Is this<br>
>> allowed?<br>
>><br>
>> 2. Are php notices fail material?<br>
>><br>
>> 3. What about clearing h2 tags in the post content? This theme does not<br>
>> do<br>
>> this and it makes for messy (IMHO) display of the floated image tests.<br>
>> Is<br>
>> this something that I should report as a suggestion? Is it grounds for<br>
>> failure?<br>
>><br>
>> 4. Did I go far enough on the review? Too far? I would have failed the<br>
>> theme for it not passing theme check requirements. Should I have just<br>
>> stopped here?<br>
>><br>
>><br>
>> ==================<br>
>> REVIEW STARTS HERE<br>
>> ==================<br>
>><br>
>> Theme Check Plugin - Required<br>
>> -----------------------------<br>
>> The theme doesn't have post pagination code in it. Use posts_nav_link()<br>
>> or<br>
>> paginate_links() or next_posts_link() and previous_posts_link() to add<br>
>> post pagination.<br>
>><br>
>><br>
>> Theme Check Plugin - Recommended<br>
>> --------------------------------<br>
>> 1. Text domain problems in search.php. You have not included a text<br>
>> domain! Line 12: $count = $allsearch->post_count; _e('');<br>
>> 2. Theme URI: is missing from your style.css header.<br>
>><br>
>><br>
>> Theme Unit Test: Many Categories<br>
>> --------------------------------<br>
>> No categories are displayed in single post view. Line 27 of index.php<br>
>> has<br>
>> what I believe to be a logic error where get_the_tag_list() is checked<br>
>> twice: if ( get_the_tag_list() || get_the_tag_list() ). I would suggest<br>
>> checking for categories as well here.<br>
>><br>
>><br>
>> Theme Unit Test: Post Format Test: Image (Linked)<br>
>> -------------------------------------------------<br>
>> The image in this post breaks out of the content area and spills into<br>
>> the<br>
>> sidebar.<br>
>><br>
>><br>
>> Attachment Views<br>
>> ----------------<br>
>> Clicking on one of the images from the "Post Format Test: Gallery" post<br>
>> will bring you to the view that I am talking about here. There are no<br>
>> links to navigate to the previous or next image in the gallery. There<br>
>> are<br>
>> also two php notices generated in this template:<br>
>><br>
>> Notice: Undefined variable: cmpvalues in core\plugins\seo-features.php<br>
>> on<br>
>> line 409<br>
>> Notice: Undefined offset: 0 in<br>
>> core\plugins\breadcrumbs\yoast-breadcrumbs.php on line 211<br>
>><br>
>> I would suggest that you create an attachment.php template that will<br>
>> handle the display of all attachments. The following link shows the<br>
>> template heirarchy that is used for these views: <a<br>
>> href="<a href="http://codex.wordpress.org/Template_Hierarchy#Attachment_display" target="_blank">http://codex.wordpress.org/Template_Hierarchy#Attachment_display</a><br>
>> ">Attachments</a>.<br>
>><br>
>><br>
>> Empty Result Set - php notices<br>
>> ------------------------------<br>
>> Please enter the a string that will produce no results ("123" worked for<br>
>> me) into the search form located in the upper right-hand side of the<br>
>> theme. 5 php notices are generated when there is no result set to<br>
>> display.<br>
>> They are all created by the following file on lines 404, 406 and 409.<br>
>> \3.1\wp-content\themes\wtrt-billions\core\plugins\seo-features.php.<br>
>> These<br>
>> notices are not limited to search results. They extend into any case<br>
>> where<br>
>> an empty result set is returned including: non-existant categories and<br>
>> tags.<br>
>><br>
>><br>
>> Search Results (page two only) - php notices<br>
>> --------------------------------------------<br>
>> In the event that a search query produces more than one page of results,<br>
>> page 2 always seems to display the following notice.<br>
>> "Notice: Undefined variable: cmpvalues in<br>
>><br>
>> C:\xampplite\htdocs\3.1\wp-content\themes\wtrt-billions\core\plugins\seo-features.php<br>
>> on line 409". This appears to be the same notice as reported above.<br>
>> Fixing<br>
>> one *should* fix both.<br>
>><br>
>><br>
>> Paged Navigation in Blog View<br>
>> -----------------------------<br>
>> There is no paging for the blog view. This appears to be true where the<br>
>> front page is set to display "Blog posts" as well as when a page is<br>
>> assigned as the "Posts page". I see that there is a custom page template<br>
>> (blog-template.php) which I beilive has been created to remedy this<br>
>> situation, but it will only ever fix one of these scenarios. I would<br>
>> suggest creating a new template (home.php) which WordPress will use in<br>
>> both cases listed above. Please see the following codex article for<br>
>> clarification on this:<br>
>> <a href="http://codex.wordpress.org/Template_Hierarchy#Home_Page_display" target="_blank">http://codex.wordpress.org/Template_Hierarchy#Home_Page_display</a><br>
>><br>
>><br>
>> > Here you go!<br>
>> > <a href="http://themes.trac.wordpress.org/ticket/3278" target="_blank">http://themes.trac.wordpress.org/ticket/3278</a><br>
>> > <<a href="http://themes.trac.wordpress.org/ticket/3278" target="_blank">http://themes.trac.wordpress.org/ticket/3278</a>><br>
>> > <a href="http://themes.trac.wordpress.org/ticket/3215" target="_blank">http://themes.trac.wordpress.org/ticket/3215</a><br>
>> > <<a href="http://themes.trac.wordpress.org/ticket/3215" target="_blank">http://themes.trac.wordpress.org/ticket/3215</a>><br>
>> > <a href="http://themes.trac.wordpress.org/ticket/3294" target="_blank">http://themes.trac.wordpress.org/ticket/3294</a><br>
>> ><br>
>> > <<a href="http://themes.trac.wordpress.org/ticket/3294" target="_blank">http://themes.trac.wordpress.org/ticket/3294</a>>There are 3 to start<br>
>> with.<br>
>> > Have fun!<br>
>> ><br>
>> > Chip<br>
>> ><br>
>> > On Sat, Mar 26, 2011 at 7:20 PM, <<a href="mailto:michael@mfields.org">michael@mfields.org</a>> wrote:<br>
>> ><br>
>> >> Opps, forgot about that part its mfields all lowercase.<br>
>> >><br>
>> >> Thanks!<br>
>> >><br>
>> >> > Hey Mike, what's your WPORG username (case-sensitive)?<br>
>> >> ><br>
>> >> > Chip<br>
>> >> ><br>
>> >> > On Sat, Mar 26, 2011 at 6:57 PM, <<a href="mailto:michael@mfields.org">michael@mfields.org</a>> wrote:<br>
>> >> ><br>
>> >> >> Please hook me up with a theme or two for review. Can I keep you<br>
>> guys<br>
>> >> >> posted of any questions I have along the way? I'll try to condense<br>
>> >> them<br>
>> >> >> into as few emails as possible.<br>
>> >> >><br>
>> >> >> Thanks!<br>
>> >> >><br>
>> >> >> -Mike<br>
>> >> >><br>
>> >> >> _______________________________________________<br>
>> >> >> theme-reviewers mailing list<br>
>> >> >> <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> >> >> <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>> >> >><br>
>> >> > _______________________________________________<br>
>> >> > theme-reviewers mailing list<br>
>> >> > <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> >> > <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>> >> ><br>
>> >><br>
>> >><br>
>> >> _______________________________________________<br>
>> >> theme-reviewers mailing list<br>
>> >> <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> >> <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>> >><br>
>> > _______________________________________________<br>
>> > theme-reviewers mailing list<br>
>> > <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> > <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>> ><br>
>><br>
>><br>
>> _______________________________________________<br>
>> theme-reviewers mailing list<br>
>> <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
>> <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
>><br>
> _______________________________________________<br>
> theme-reviewers mailing list<br>
> <a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
> <a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
><br>
<br>
<br>
_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
</div></div></blockquote></div><br>