Add Ammonite Theme plugin.

[Godoy0722]

That's a theme designed by the Geological Survey of Denmark and Greenland and contributed to the OJS community. It actually have releases for OJS 3.4 and 3.5.

[Godoy0722]

Hello there @bozana . I'm pinging you on this one as well as this theme is also intended to be distributed on our plugin gallery. I appreciate if you could have a look on it as well!

[bozana]

Hi @Godoy0722, oh, a huge theme :-)
I took a look at PHP classes and smarty templates. I am not a good reviewer for JS, CSS, etc, so maybe @jardakotesovec could take a look at that part (above all for possible security issues)?

I noticed some escaping issues in the Smarty templates.

Here a few examples:

• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/plugins/generic/htmlArticleGalley/templates/display.tpl#L40: I believe publication title is an HTML so probably strip_unsafe_html needs to be used.
• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/frontend/components/article_components/license_block.tpl#L39: also here strip_unsafe_html should be used. (I see it is also not escaped in the default theme :-O)
• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/frontend/components/article_authors_section.tpl#L25: volume and year are not escaped.
• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/frontend/components/breadcrumbs_article.tpl#L38: not escaped
• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/frontend/components/article_authors_section.tpl#L85: affiliation not escaped
• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/frontend/components/footer.tpl#L27: $activeTheme->getOption('footerClientLogoLink') not escaped
• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/frontend/components/footer.tpl#L57: strip_unsafe_html needs to be used. (I see the $pageFooter is also not escaped in the default theme :-O).
• https://github.com/Godoy0722/ammoniteTheme/blob/stable-3_5_0/templates/frontend/components/article_authors_section.tpl#L86: missing quotes around {$author->getFullName()|escape} -- could lead to wrong parsing.
• etc.

Could you please review all Smarty templates and apply the appropriate escaping:

• |escape for plain-text values
• |strip_unsafe_html for rich-text / HTML content fields
• |escape on all HTML attribute values, and make sure attribute values are always wrapped in quotes

Thanks a lot!

[bozana]

Hi @Godoy0722, as in smwImmersion theme: Could we self-host the font files? -- because of GDPR.

[Godoy0722]

Hello again @bozana ! I just made a comprehensive review on all smarty template files on this theme, and I made all escape/security improvements I found other than your suggestions. I made it on both versions, for 3.4 and 3.5. Would you mind having a look on it?

Regarding the fonts, I'll work on it right now and put a new commit until tomorrow so I'm up to date with all these requests until now! Thank you again!

[Godoy0722]

Hello @bozana , me again! I added a new commit on the theme for both versions so we self-host the fonts as you suggested! Please feel free to have a look and let me know if we're good to go on this merge now! Thank you

[kaitlinnewson]

Hi @Godoy0722, I tried uploading the 3.5 release package linked here to my local install and have display issues. Is that expected? E.g. here's what it looks like after installing, enabling, and setting this as the theme and clearing the template cache (no settings configured):

[kaitlinnewson]

Okay I see the issue - this theme requires the Health Sciences theme to also be enabled to function, so is sort of a "child theme" to that - at minimum that should be documented in the description, but I'm not sure what the best practice is for this kind of dependency.

[Godoy0722]

Hello @kaitlinnewson . Thanks for also reviewing my theme! I just implemented your request. Now, the documentation has a note telling that the theme depends on the Health Sciences theme, and if the user tries to select the ammonite without the Health Sciences installed and enabled, it will throw a notification on the screen to notify the user about this matter. Would you mind reviewing it again please? I also made the same approach for the SMWImmersion theme