WordPress.org

Make WordPress Themes

Opened 2 years ago

Closed 2 years ago

#25660 closed theme (not-approved)

THEME: Liro – 1.1.3

Reported by: Sazaj Owned by: laurelfulford
Priority: new theme Keywords: theme-liro
Cc: samuelzajakala@…

Description

Liro - 1.1.3

Awesome WordPress theme with many well known WordPress plugins. We have made this theme to be used for e-commerence,

Theme URL - http://liro.lab-404.com
Author URL - http://www.lab-404.com

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

History:

Ticket Summary Status Resolution Owner
#25660 THEME: Liro – 1.1.3 closed not-approved laurelfulford

(this ticket)


https://themes.svn.wordpress.org/liro/1.1.3/screenshot.png

Change History (10)

#1 @Sazaj
2 years ago

Hi,
Just wanted to say that our theme preview does not work now. I will try to get it working as soon as possible!

Last edited 2 years ago by Sazaj (previous) (diff)

#2 @laurelfulford
2 years ago

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

#3 @laurelfulford
2 years ago

Hello!

I’ve checked over your theme, and included some feedback below. The 'Required' items will need to be done before the theme can be approved.

The 'Recommended' items are only recommendations to improve your theme, and do not need to be completed to have your theme approved.

Once the 'Required' items are completed, the updated theme can be uploaded here: https://wordpress.org/themes/upload/


Required

Code

  • Sanitize everything. I spotted the following spots that need escaping; there may be more:
    • category.php, line 11 - get_author_posts_url() needs escaping
    • content-single.php, line 4 - get_author_posts_url() needs escaping
    • content.php, line 12 - get_author_posts_url() needs escaping
    • content.php, lines 39, 46 - get_the_excerpt() should be escaped, or can be switched for the_except(), since that function doesn’t need escaping
    • functions.php lines 232, 239, 244, etc. - escape get_theme_mod() values
    • searchform.php, line 1 - home_url() needs to be escaped
  • Support the following WordPress-generated CSS classes:
    • screen-reader-text - needs to be included in style.css

If you’d like to learn more about escaping, Frank Klein is writing a great series - the first post is here: part1.

There is also some information in the Codex.

Core Functionality and Features

  • Include comments_template()
    • Technically is included, but only in index.php, so it's not visible on posts
  • The theme tags and description must match the what the theme actually does in respect to functionality and design.
    • Description doesn't seem accurate. For example, mentions theme includes plugins (the theme shouldn't include plugins, so one option would be to update to say what plugins are supported, or remove).

Favicons

  • If implemented, disable favicons by default and have the ability for users to override. Function my_favicon() in functions.php, line 85, needs to be updated.

Language

  • All theme text strings are to be translatable. I found some that were not marked for translation - there may be more than this:
    • archive.php, lines 25, 28, 30, 32, 34, 47
    • category, lines 11, 41
    • comments.php, line 14, 39, 46, 56, 62
    • content-aside.php - line 3, ‘@‘ should be marked for translation (symbol may not exist/be understood the same in all languages)
    • content-link.php - line 3, ‘@‘ should be marked for translation
    • content-single.php, line 4
    • content.php, lines 12, 40, 47
    • footer-template.php - line 16, 17, 26
    • footer.php - line 54, 55
    • front-page.php, line 16
    • functions.php, lines 95, 121, 139, 139, 148, 157
    • header.php, line 35, 53, 55
    • index.php, line 21
    • page-portfolio.php, line 31
    • page.php, line 18
    • search.php, lines 15, 25
    • searchform.php, line 2, 4
    • single.php, line 26
    • special-template.php, line 17, 18, 27
  • Use a single unique theme slug – as the theme slug appears in style.css. If it uses a framework then no more than 2 unique slugs. I’ve spotted some places where different theme slugs are used - there may be more:
    • comments.php, line 7 - textdomain is ‘kubrick'
    • functions.php, line 66, 67, 93, 109, etc - textdomain needs to be updated to theme name
  • Please generate a POT file for the theme and include it in a 'languages' folder. There's some more information about how to do this here: http://codex.wordpress.org/I18n_for_WordPress_Developers#POT_files

Licensing

  • Declare licenses of any resources included such as fonts or images.
    • Need license information for FontAwesome, Google fonts in the readme.txt

Options and Settings

  • Prefix all options, custom functions, custom global variables and custom constants with the theme-slug. There are some spots that need to be updated, including:
    • functions.php - all custom functions need to be prefixed with the theme name
    • functions.php - custom image sizes should be prefixed with the theme name
    • functions.php, settings should be prefixed with the theme name instead of ‘themeslug’, ‘lwp', etc.

Screenshot

  • The Screenshot should be of the actual theme as it appears with default options, not a logo or mockup. A new one should be creating removing the Jetpack reference, and the three buckets with icons at the bottom (unless there is a specific way to create this in the theme - if yes, please include in documentation).

Stylesheets and Scripts

  • No hard coding of scripts, styles and Favicons unless a browser workaround script. Everything should be enqueued.
    • header.php, Font Awesome and Google Fonts should be enqueued.
    • in functions.php, everything can be enqueued in the same function, including the comment-reply script (see _s for an example: https://github.com/Automattic/_s/blob/master/functions.php#L114-L125)
    • functions.php, line 85: if including a favicon, it should be enqueued. Right now the
    • functions.php, line 219 - output from Liro_customize_css() function should be enqueued
  • Include all scripts and resources it uses rather than hot-linking. The exception to this is Google libraries.
    • FontAwesome should be included in the theme
  • If a tag is used in style.css the theme should support that feature or adhere to what that tag stands for. For example custom background or header.
    • Where is green used in the theme?
    • Theme doesn’t appear to support custom-header; the tag should be removed, or support added
    • Theme doesn’t seem to be fully responsive - the menu is, but the content isn't

Recommendations

General

  • Would recommend using a lowercase text domain, function prefix throughout theme.

content.php

  • line 37 - use || instead of OR
  • line 39, 46 - Use the_excerpt(), rather than echo get_the_excerpt() (since the_excerpt() is already escaped)

footer-template.php and special-template.php

  • Consider making disclaimer text something a user can edit through WordPress, rather than through the theme.

functions.php

  • Question: are all the functions in this file used (eg. get_top_ancestor_id(), has_children())? I couldn’t spot them, but I might have missed something!
  • Question: out of curosity, what are the_tags(), the_posts_pagination() functions doing in this file?

header.php

  • Would recommend not hardcoding the ‘View Cart’ link in the menu

readme.txt

style.css

  • Theme URI returns a 404 error (you've already noted this above!)

Next Steps

Once the 'Required' items are complete, you can upload the theme again using the following link, and the review can continue.

https://wordpress.org/themes/upload/

Also if you have any questions, just let me know!

Best,

Laurel

#4 @laurelfulford
2 years ago

Hello!

I should have added in my review - it's important to follow up on open tickets at least once every seven days, even if it's just to add a comment that you need more time to work on the theme. Tickets are typically closed if they become inactive for more than seven days.

Let me know if you have any questions at all about this!

Best,

Laurel

#5 @Sazaj
2 years ago

Hey!

I'm totally sorry, I have a lot of trouble with my internet connection right now, because I'm moving to another country and my internet was shut down 3 weeks too fast, I'm really, really sorry about it.

I had also managed to get a server up and running, but there is no point of it since the internet is down...

Also as a beginner in PHP, while having a some good knowledge of WordPress, I lost myself at

"Sanitize everything. I spotted the following spots that need escaping; there may be more: "

as I don't really know what you mean by Sanitizing. Is there anything that could explain to me "Sanitizing"?

Thanks a lot for spending your time on my Theme!

Looking forward to hear from you,
Samuel.

#6 @laurelfulford
2 years ago

Hi Samuel!

Thanks for letting me know what's up - sounds like things have been pretty crazy for you! I will keep that in mind for this ticket, to make sure you have some more wiggle room. Moving can be nuts!

“Sanitize everything” refers to making sure nothing malicious can be added through your theme. I linked to a couple resources in the original review, but I can give you a super-super beginner intro:

For most themes, the main sanitization you’ll need to do is escaping. Escaping is added where something is echoed, to make sure nothing bad is being passed (like malicious JavaScript). In WordPress, some functions already include escaping (like the_content(), or the_permalink()), so they are OK to leave as is.

Other functions (like get_author_posts_url(), or get_theme_mod()) don’t include escaping, so it has to be manually added to the theme instead. It’s recommended that the escaping is added at the last possible moment, so it’s easy to tell that something has been escaped without digging backwards in the code.

Some escaping functions you’ll probably use a lot are:

  • esc_url() - this should be used on anything that’s a URL (so a src or href value)
  • esc_attr() - this should be used on anything that’s in an attribute, like a title, ID, class, etc. in an HTML tag.
  • esc_html() - this will encode all HTML tags, turning < into &lt;, > into &gt;, etc. It should be used on content that should not include HTML.
  • wp_kses() - this will remove HTML tags, except the ones that you’ve specifically listed. It can be a resource heavy function, so should be used sparingly.

The Codex is a good resource for basic escaping functions (including the ones above). Frank Klein’s series on building secure themes (four parts, here) is a great resource to learn more about why and how to sanitize different things, not just adding escaping.


These are the areas that need escaping in your theme, along with a recommendation for the function to use:

category.php, line 11 - get_author_posts_url() should be escaped with esc_url() because it's a URL. esc_url() will make sure only things that should be included in a URL are there. The whole thing would look like: echo esc_url( get_author_posts_url(get_the_author_meta('ID’)) )

content-single.php, line 4 - get_author_posts_url() - same as above

content.php, line 12 - get_author_posts_url() - same as above

content.php, lines 39, 46 - get_the_excerpt() should be escaped. In this case, I would just recommend changing it from echo get_the_excerpt() to the_excerpt(). the_excerpt() already includes escaping, and won’t need to be echoed (since that’s already done in that function). Alternatively, since excerpts don’t include html, you could also use esc_html(), like echo esc_html( get_the_excerpt() )

functions.php - when get_theme_mod() is echoed needs to be escaped. What escaping function you use will depend on what should be returned. In this case, on lines 225, 232, 239 and 244, get_theme_mod() returns a hexadecimal number. Something like esc_html() would work fine here, because it will allow a hexadecimal colour to display, but will strip out things that shouldn’t be there (like HTML):

line 225 echo esc_html( get_theme_mod('lwp_link_color') )

line 232 - echo esc_html( get_theme_mod('lwp_link_color') )

line 239 - echo esc_html( get_theme_mod('lwp_btn_color') )

line 244, echo esc_html( get_theme_mod('lwp_btn_hover_color') )

searchform.php, line 1 - home_url() needs to be escaped. It can be escaped with esc_url(), like echo esc_url( home_url() )


I found escaping super confusing when I first started doing it (still do sometimes, to be honest!). Let me know if you have any questions about the above :)

Laurel

#7 @Sazaj
2 years ago

Hey Laurel!

Thank you so much for understanding my troubles and giving me more wiggle room! I appreciate it!

Also, thank you a lot for showing me some examples of how to sanitize everything. It will help me a lot. I will work offline, so I can get some changes done and once I will move out ( Will be in the country in 17th of August. I'll have everything sorted on the 18th of August so I'll be able to upload the theme then) I will be able to update the theme!

Thank you for your time!

Have a great day,
Samuel.

#8 @laurelfulford
2 years ago

Hi Samuel,

Hope things are going well! I just wanted to check in on this review, to see where things are at.

Just let me know if you have any questions as you work through the list!

Thanks!

Laurel

#9 @Sazaj
2 years ago

Hi Laurel,

Unfortunately I have to stop with Liro for now. You can close this theme, as I have got no time to work on it and probably will have no time to work on it.

I'm really sorry.

Regards,
Samuel.

#10 @laurelfulford
2 years ago

  • Resolution set to not-approved
  • Status changed from reviewing to closed

It's not a problem at all, Samuel! Thank you for letting me know :)

I will close this ticket for now, but if you have a chance to move forward with the theme in the future, please re-upload it and a new ticket will be created.

https://wordpress.org/themes/upload/

Best,

Laurel

Note: See TracTickets for help on using tickets.