WordPress.org

Make WordPress Themes

Change History (25)

#1 @poena
2 months ago

  • Keywords changed from theme-reader-wp-lite, accessibility-ready to theme-reader-wp-lite accessibility-ready
  • Owner set to poena
  • Status changed from new to reviewing

#2 @poena
2 months ago

Hi
I have reviewed version 1.0.4, and there has been some improvements since I last saw the theme,
but there are still things that needs to be fixed before your theme can be approved.

If you are not sure what I meant, please ask before submitting a new version.

Required
The Display Site Title and Tagline option in the customizer is not working correctly.
It only hides the tagline, not the site title.

In customizer.js, line 39-40, you need to remove the upgrade part:
1) Sections/ panels and settings need to be added using the customizer api, not by JS injection.
2) The text is not tanslation ready.
Please see this example code that you can use instead: https://github.com/justintadlock/trt-customizer-pro
-Don't forget to prefix / rename the example names if you decide to use it.

The readme.md file still refers to underscores, it needs to be updated or removed.
No minification of scripts or files unless you provide original files.
There are javascripts that are missing the original, non minified files.

In readme.txt, you are sying that "All the JS are licensed under GPLv2 or later"
This is not correct, you need to look up each of the third party scripts and correct it.
Your users are relying on you, this needs to be correct or you can cause licensing problems for them in the future.

All untrusted data should be escaped before output.
All instances of home_url needs to be escaped as esc_url( home_url( '/' ) );
Please never do this without escaping: echo get_theme_mod, it is not safe, trust no one.
See https://developer.wordpress.org/themes/theme-security/data-sanitization-escaping/

Enqueued google fonts needs to be prefixed.
-You have only added a prefix for one of the 3 fonts.

There is still text that is missing translation functions.
You need to open each file and add translation funtions to all the texts.

Do not add_theme_support in functions.php, only to remove it in customizer.php. Remove the add_theme_support instead.

Remove any and all code, files or folders that are not used.
There are empty functions in template-tags.php. Delete apple-touch-icon.

wp-bootstrap-navwalker.php
There are a couple of places where esc_attr is used between html tags, not inside actual html attributes. Replace these with esc_html.
The text needs to be translation ready: esc_attr( 'Add a menu', )

Theme URI is optional.
If used, it must be about the theme we’re hosting on WordPress.org.
If the URI is a demo site, the content must be about the theme itself and not test data.

#3 @Litonice13
2 months ago

Hello @poena,
Thanks for your details review. Mostly I've fixed.
I've confusion about this line
"Do not add_theme_support in functions.php, only to remove it in customizer.php. Remove the add_theme_support instead.
Remove any and all code, files or folders that are not used."

Can you please let me know exactly what should I've to do. If possible with an example code.
Thanks

#4 @poena
2 months ago

Hi
1)
In functions.php, line 71, you have add_theme_support( 'custom-header'
In custom-header.php line 20, you also have a duplicate add_theme_support( 'custom-header'

But in customizer.php, line 32 and 33, you have:

$wp_customize->remove_control( 'header_image' );
$wp_customize->remove_control( 'header_textcolor' );

So you are adding the custom header, only to remove it. Remove all 3 of these codes.
You can remove custom-header.php, because there are no other functions in the file.

2)
Remove any and all code, files or folders that are not used

I trust that you, as the author, knows what code is and is not used. Remove all code, files or folders that are not used.
There are empty functions in template-tags.php. Delete them if you don't plan to use them.
Delete apple-touch-icon.

Edit: spelling

Last edited 2 months ago by poena (previous) (diff)

#5 @themetracbot
2 months ago

  • Summary changed from THEME: Reader WP Lite – 1.0.4 to THEME: Reader WP Lite – 1.0.5

Reader WP Lite - 1.0.5

Reader WP Lite is a Personal Blogging WordPress Theme

Theme URL - http://demo.prowptheme.com/reader/
Author URL - https://prowptheme.com

Trac Browser - https://themes.trac.wordpress.org/browser/reader-wp-lite/1.0.5

SVN - https://themes.svn.wordpress.org/reader-wp-lite/1.0.5
ZIP - https://wordpress.org/themes/download/reader-wp-lite.1.0.5.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reader-wp-lite/1.0.4&new_path=reader-wp-lite/1.0.5

History:

Ticket Summary Status Resolution Owner
#42730 THEME: Reader WP Lite – 1.0.1 closed not-approved poena
#44377 THEME: Reader WP Lite – 1.0.3 closed not-approved sampression
#45893 THEME: Reader WP Lite – 1.0.9 closed live poena

(this ticket)

#46750 THEME: Reader WP Lite – 2.0.0 closed live themetracbot


https://themes.svn.wordpress.org/reader-wp-lite/1.0.5/screenshot.jpg
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.

#6 @Litonice13
2 months ago

Hello @poena,
Thanks for clearing the issue. I've fixed your given issues.

  1. Display Site Title and Tagline option in Customizer fixed
  2. Upgrade Notice in customizer.js file removed
  3. readme.md file removed
  4. In readme.txt file updated with JS and CSS files License details. Image sources with License also given.
  5. Original files provided for minification files.
  6. untrusted data escaped with esc_url( home_url(/) )
  7. all get_theme_mod data escaped properly
  8. Enqueued google fonts prefixed and corrected format
  9. add_theme_support() issue with customizer fixed. Removed unnecessary file custom-header.php
  10. Empty function removed from template-tags.php file
  11. Translation ready : esc_attr( 'Add a menu', 'reader-wp-lite') fixed. Also fixed html tags on this wp-bootstrap-navwalker.php file
  12. Theme URI fixed and also made Demo of Reader WP Lite

Hope everything is fine now.

Thanks

#7 @poena
8 weeks ago

Hi
This is better, but there are still things that I mentioned that has not been fixed.

Required

Customizer
There seem to be some misunderstanding about the header settings, and how custom controls work.
The header text color option, and the display and hide site title and tagline, are only available if your theme has support for custom header, and that was removed in this version.

I don't fully understand what you want to do, if you explain it to me then I can probably help you.

Your two settings, display_logo and display_site_title, are not visible in the customizer because you did not add a section arg to "add_control".
Logos should be visible when the user has added one. To not show the logo, the user can simply remove the image, so this setting is not really needed?
I added a section to the add_control to be able to test them, but they are not working very well,
neither of the changes show in the customizer, unless I refresh the page.

The option says Display Site Title, not hide side title, so I don't udnerstand the display:none.

sanitize_text_field should be used for text fields, not for check boxes. You need to validate the check boxes before saving.
Examples: https://github.com/WPTRT/code-examples/blob/master/customizer/sanitization-callbacks.php

Remove these lines:
$wp_customize->get_setting( 'header_textcolor' )->transport = 'postMessage';
$wp_customize->remove_control( 'header_image' );
$wp_customize->remove_control( 'header_textcolor' );


No minification of scripts or files unless you provide original files.
-Missing a non minified version of modernizr.

In readme.txt,
This does not look correct, is this meant to say "based on Underscores"?

Missing license for the font awesome font files.

Neither of these images are used in the screenshot. The information about the image that is used in the screenshot seems to be missing.
https://unsplash.com/photos/KAsjiTRuihk
http://www.uhdwallpapers.org/2015/05/girl-listen-in-headphones-good-music.html
https://images.unsplash.com/reserve/L55hYy77SLqb6zeTMlWr_IMG_9035.jpg
https://unsplash.com/photos/6PF6DaiWz48
https://unsplash.com/photos/FjAD28N8-IQ

There is still text that is missing translation functions.
You need to open each file and add translation funtions to all the texts.


wp-bootstrap-navwalker.php
esc_html() should be used between html tags.
esc_attr() should be used inside html attributes.
Here is a list of html attributes: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes

You replaced some of the esc_attr() that was used correctly inside attributes, example:
This is correct: id="' . esc_attr( $id ) . '"'
This is the change you made, that is not correct: id="' . esc_html( $id ) . '"'

esc_attr( 'Add a menu' , 'reader-wp-lite')
This is not correct because:
esc_attr() is not a translation function, while esc_attr__() is.
But since this is not inside an attribute, you should use esc_html__().

https://developer.wordpress.org/reference/functions/esc_html__/
https://developer.wordpress.org/reference/functions/esc_attr/
https://developer.wordpress.org/reference/functions/esc_attr__/


Recommended
In header.php, the pingback should be conditional. See this example from Twenty Seventeen:

/**
 * Add a pingback url auto-discovery header for singularly identifiable articles.
 */
function twentyseventeen_pingback_header() {
	if ( is_singular() && pings_open() ) {
		printf( '<link rel="pingback" href="%s">' . "\n", get_bloginfo( 'pingback_url' ) );
	}
}
add_action( 'wp_head', 'twentyseventeen_pingback_header' );

#8 @themetracbot
8 weeks ago

  • Summary changed from THEME: Reader WP Lite – 1.0.5 to THEME: Reader WP Lite – 1.0.6

Reader WP Lite - 1.0.6

Reader WP Lite is a Personal Blogging WordPress Theme

Theme URL - http://demo.prowptheme.com/reader/
Author URL - https://prowptheme.com

Trac Browser - https://themes.trac.wordpress.org/browser/reader-wp-lite/1.0.6

SVN - https://themes.svn.wordpress.org/reader-wp-lite/1.0.6
ZIP - https://wordpress.org/themes/download/reader-wp-lite.1.0.6.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reader-wp-lite/1.0.5&new_path=reader-wp-lite/1.0.6

History:

Ticket Summary Status Resolution Owner
#42730 THEME: Reader WP Lite – 1.0.1 closed not-approved poena
#44377 THEME: Reader WP Lite – 1.0.3 closed not-approved sampression
#45893 THEME: Reader WP Lite – 1.0.9 closed live poena

(this ticket)

#46750 THEME: Reader WP Lite – 2.0.0 closed live themetracbot


https://themes.svn.wordpress.org/reader-wp-lite/1.0.6/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.

#9 @Litonice13
8 weeks ago

Hello @poena,

Sorry for the confusions.

Customizer:

  1. On very first Development of this Theme I used few controls over default Title, Logo and Tagline controls. I wanted to remove default settings and make my own settings. That's why things were messed up.

Anyway, actually I don't need those settings like Header text color, display logo. I've removed those unnecessary codes.

  1. sanitize_text_field used only for text fields.
  1. Non minified version of Modernizer file added. I didn't recognize this, that's why I missed it.
  1. "based on Underscores" text removed from readme.txt file
  1. Screenshot has been changed to present Pictures given on readme.txt file
  1. wp-bootstrap-navwalker.php file fixed with escaped validation
  1. Pingback issue fixed by your reference.
  1. I've checked with Theme Check Plugin. I didn't find text with missing translation. Can you please let me know the easy way to find Strings those are not translatable.

Thanks

#10 @poena
8 weeks ago

Hi

I reset my install and now when I activate the new version of the theme I see the following notices:
Notice: Trying to get property of non-object in themes/reader-wp-lite/inc/wp_bootstrap_navwalker.php on line 66
Notice: Trying to get property of non-object in themes/reader-wp-lite/inc/wp_bootstrap_navwalker.php on line 85
Notice: Trying to get property of non-object in themes/reader-wp-lite/inc/wp_bootstrap_navwalker.php on line 104
Notice: Trying to get property of non-object in themes/reader-wp-lite/inc/wp_bootstrap_navwalker.php on line 118
Notice: Trying to get property of non-object in themes/reader-wp-lite/inc/wp_bootstrap_navwalker.php on line 118
Notice: Trying to get property of non-object in themes/reader-wp-lite/inc/wp_bootstrap_navwalker.php on line 119
Notice: Trying to get property of non-object in themes/reader-wp-lite/inc/wp_bootstrap_navwalker.php on line 120

First I thought this was related to the new escaping, but, it does not correspond to these lines...

Warning: sprintf(): Too few arguments in themes/reader-wp-lite/comments.php on line 30

wp-bootstrap-navwalker.php
There are still places where esc_html() is used in attributes.
Line 72, 114

You removed the site title setting (display_site_title), but you are still refering to it on

themes\reader-wp-lite\header.php:
   55:         <?php if( get_theme_mod( 'display_site_title' , '0' ) == '1') { ?>
themes\reader-wp-lite\inc\customizer.php:
   15: $showttlslogan = get_theme_mod('display_site_title');
2 matches across 2 files

I am not sure how to explain it better :(
Yes sanitize_text_field is used for text fields, but not for links. For links, please use esc_url_raw().

https://vip.wordpress.com/documentation/validating-sanitizing-escaping/#sanitizing-cleaning-user-input
https://developer.wordpress.org/reference/functions/sanitize_text_field/
https://developer.wordpress.org/reference/functions/esc_url_raw/

Theme Check can't find text that is missing translation functions, you need to manually open each file and look at the texts. Or you can try using the piglatin plugin, but looking through the files is probably faster.
https://wordpress.org/plugins/piglatin/
Here are some:
functions.php Line 170 placeholder="Search"
template-tags.php, Line 129, comments_number( 'No Comment', '1 Comment', '% Comments' );
content-aside.php, content-search.php: Continue reading ...

You have a typo in your footer: Develpoed

#11 @poena
8 weeks ago

About the links in the customizer, use type url instead of text
Example: change

	$wp_customize->add_control(	'facebook', 
		array(
			'label'    => esc_html__( 'Facebook Username', 'reader-wp-lite' ),
			'section'  => 'social_section',
			'settings' => 'facebook',
			'type'     => 'text'
		)
	);

to

	$wp_customize->add_control(	'facebook', 
		array(
			'label'    => esc_html__( 'Facebook Username', 'reader-wp-lite' ),
			'section'  => 'social_section',
			'settings' => 'facebook',
			'type'     => 'url'
		)
	);

#12 @poena
8 weeks ago

One more question, did you mean to include the accessibility-ready tag? I did not do a accessibility review yet.

#13 @poena
7 weeks ago

Hi, it has been 7 days, do you plan to submit an update?

#14 @Litonice13
7 weeks ago

Hello @poena,
I'm very sick, lying on the bed from last 10 days. Please don't close this ticket, I will try my best updating within 3-4 days.
Thanks

#15 @poena
7 weeks ago

Ohw, feel better soon!

#16 @themetracbot
6 weeks ago

  • Keywords accessibility-ready removed
  • Summary changed from THEME: Reader WP Lite – 1.0.6 to THEME: Reader WP Lite – 1.0.7

Reader WP Lite - 1.0.7

Reader WP Lite is a Personal Blogging WordPress Theme

Theme URL - http://demo.prowptheme.com/reader/
Author URL - https://prowptheme.com

Trac Browser - https://themes.trac.wordpress.org/browser/reader-wp-lite/1.0.7

SVN - https://themes.svn.wordpress.org/reader-wp-lite/1.0.7
ZIP - https://wordpress.org/themes/download/reader-wp-lite.1.0.7.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reader-wp-lite/1.0.6&new_path=reader-wp-lite/1.0.7

History:

Ticket Summary Status Resolution Owner
#42730 THEME: Reader WP Lite – 1.0.1 closed not-approved poena
#44377 THEME: Reader WP Lite – 1.0.3 closed not-approved sampression
#45893 THEME: Reader WP Lite – 1.0.9 closed live poena

(this ticket)

#46750 THEME: Reader WP Lite – 2.0.0 closed live themetracbot


https://themes.svn.wordpress.org/reader-wp-lite/1.0.7/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.

#17 @Litonice13
6 weeks ago

Hello @poena,

  1. wp_bootstrap_navwalker.php error issues fixed
  2. Comments.php sprintf() function issue fixed
  3. Removed title settings (display_site_title) from header.php
  4. sanitize_text_field and esc_url_raw() issue fixed
  5. functions.php, template-tags.php, content-aside.php, content-search.php issues fixed
  6. Typo fixed on footer
  7. On customizer used type url instead of text
  8. accessibility-ready removed from style.css file

Hope everything is fine now.
Thanks

#18 @poena
6 weeks ago

Hi
I apologize for the huge mistake I made in the last review.
Obviousely if you are asking the user to only add the username, and not the full link, then yes you should use the text type and sanitize_text_field.
It's even right there in the example... I'm sorry!


The PHP notices have been fixed.

You changed esc_html__() to esc_html() in a few places, but esc_html does not translate the text:
https://developer.wordpress.org/reference/functions/esc_html/

Use esc_attr__ in html attributes:
placeholder="' . esc_html__( 'Search', 'reader-wp-lite' ) . '"

Last edited 6 weeks ago by poena (previous) (diff)

#19 @themetracbot
6 weeks ago

  • Summary changed from THEME: Reader WP Lite – 1.0.7 to THEME: Reader WP Lite – 1.0.8

Reader WP Lite - 1.0.8

Reader WP Lite is a Personal Blogging WordPress Theme

Theme URL - http://demo.prowptheme.com/reader/
Author URL - https://prowptheme.com

Trac Browser - https://themes.trac.wordpress.org/browser/reader-wp-lite/1.0.8

SVN - https://themes.svn.wordpress.org/reader-wp-lite/1.0.8
ZIP - https://wordpress.org/themes/download/reader-wp-lite.1.0.8.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reader-wp-lite/1.0.7&new_path=reader-wp-lite/1.0.8

History:

Ticket Summary Status Resolution Owner
#42730 THEME: Reader WP Lite – 1.0.1 closed not-approved poena
#44377 THEME: Reader WP Lite – 1.0.3 closed not-approved sampression
#45893 THEME: Reader WP Lite – 1.0.9 closed live poena

(this ticket)

#46750 THEME: Reader WP Lite – 2.0.0 closed live themetracbot


https://themes.svn.wordpress.org/reader-wp-lite/1.0.8/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.

#20 @Litonice13
6 weeks ago

Hello @poena,
No need to be sorry :)

I've fixed "sanitize_text_field" issue. I was asking for the Username only. I also forgot to mention this.

I've fixed translation issues with esc_html()

Hope it's fine now.

Thanks

#21 @poena
5 weeks ago

Hi
Lets see if we can get this right once and for all :)

template-tags.php
Line 34: esc_attr() is used between html tags.
'<span class="author vcard"><a class="url fn n" href="' . esc_url( get_author_posts_url( get_the_author_meta( 'ID' ) ) ) . '">' . esc_attr( get_the_author() ) . '</a></span>'

Line 128: the_time() is already echoed and does not need to be echoed again.
Reference: https://developer.wordpress.org/reference/functions/the_time/
<span class="date"><?php echo the_time( get_option( 'date_format' ) ); ?></span>

You only need to escape urls that are unsafe, not static links that are not meant to be changed,
for example your credit link.
For the social links, you don't need to escape the first part, (https://www.twitter.com/ etc) because it's hard coded. You only need to escape the usernames. Use esc_html for the usernames.

For the username settings, you updated the sanitize callback but the type is still set to "url".

Note:
When I added a link in the "URL of Company" option, the output was: "https://www.https://example.com".
There is no information about the format of the url.

#22 @themetracbot
5 weeks ago

  • Summary changed from THEME: Reader WP Lite – 1.0.8 to THEME: Reader WP Lite – 1.0.9

Reader WP Lite - 1.0.9

Reader WP Lite is a Personal Blogging WordPress Theme

Theme URL - http://demo.prowptheme.com/reader/
Author URL - https://prowptheme.com

Trac Browser - https://themes.trac.wordpress.org/browser/reader-wp-lite/1.0.9

SVN - https://themes.svn.wordpress.org/reader-wp-lite/1.0.9
ZIP - https://wordpress.org/themes/download/reader-wp-lite.1.0.9.zip?nostats=1

Diff with previous version: https://themes.trac.wordpress.org/changeset?old_path=reader-wp-lite/1.0.8&new_path=reader-wp-lite/1.0.9

History:

Ticket Summary Status Resolution Owner
#42730 THEME: Reader WP Lite – 1.0.1 closed not-approved poena
#44377 THEME: Reader WP Lite – 1.0.3 closed not-approved sampression
#45893 THEME: Reader WP Lite – 1.0.9 closed live poena

(this ticket)

#46750 THEME: Reader WP Lite – 2.0.0 closed live themetracbot


https://themes.svn.wordpress.org/reader-wp-lite/1.0.9/screenshot.png
Theme Check Results:

  • RECOMMENDED: No reference to add_theme_support( "custom-header", $args ) was found in the theme. It is recommended that the theme implement this functionality if using an image for the header.
  • RECOMMENDED: No reference to add_theme_support( "custom-background", $args ) was found in the theme. If the theme uses background images or solid colors for the background, then it is recommended that the theme implement this functionality.

#23 @Litonice13
5 weeks ago

Hello @poena,
You're helping me a lot.
Anyway, I've fixed your given issues.

  1. template-tags.php Line 34: esc_attr() is used between html tags issue fixed
  2. Line 128: the_time() echo issue fixed
  3. Escape issue on Social Usernames and Footer Credit link fixed
  4. Given information on "URL of Company" Option and Footer Credit Text.

Hope this time everything will be fine :)
Thanks

#24 @poena
5 weeks ago

  • Resolution set to live
  • Status changed from reviewing to closed

Hi!
Yes I believe that is it. I will set the theme live and it should show up in the directory shortly.
Time to celebrate :)

If we missed anything it can be fixed in an update.


Note: this theme is licensed under GNU General Public License v2 or later,
but the website says: "All of our themes are licensed under the GNU general public license (GPL3)"

#25 @Litonice13
5 weeks ago

Hello @poena,
Great to see that my Theme is Live :)
I will fix the License issue asap.
Thanks

Note: See TracTickets for help on using tickets.