Emailprint button templatization

[rica-carv]

Motivation and Context

Suggested option to fully allow templatization of buttons, either bootstrap output or not. Buttons can be rendered as A tags or BUTTON tags, and the HTML hardcoded inside the PHP makes the customization a lot harder than with templates. Templates can even allow the templatization at theme level, much easier without rewriting the whole PHP code to have a custom button, like for instance, with text besides icon....
If approved, this can also be the starting point of allowing templatization of e107 buttons, etc....

Even better, this code is only currently used in news, but it can be used also in forum, etc., to allow a standardization of layouts sitewide....
Just my 2 cents...

Description

Changed hardcoded HTML to a new template, and leave old code for legacy purposes commented.

How Has This Been Tested?

Tested in last github e107 code

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (a change to man pages or other documentation)

Checklist

• My code adheres to the e107 code style standard.
• I have read the contributing document.
• I have added tests to cover my changes.
• All new and existing tests pass.

[qlty-cloud-legacy]

Code Climate has analyzed commit ce99d9e and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Bug Risk	4

The test coverage on the diff in this pull request is 85.1% (80% is the threshold).

This pull request will bring the total coverage in the repository to 39.0%.

View more on Code Climate.

[e107help]

@rica-carv Pulling the email/print button markup out of emailprint_class.php and into a core template is worth doing, and the ?: fallbacks mean a site with no template override keeps its current output. Nice touch on the BC front.

Three things I'd want changed before it goes in.

The hardcoded fas prefix breaks FontAwesome 4 sites

The new template writes the markup directly:

$EMAILPRINT_TEMPLATE['ICON_EMAIL'] = "<i class='fas fa-envelope'></i>"; //$tp->toGlyph('fa-envelope',false);

toGlyph() doesn't just wrap a name in an <i>. When it's handed a plain fa- name it runs it through the shim tables for whichever FontAwesome the site is on and picks fas, far or fab accordingly (e_parse_class.php#L4279-L4300). Naming fas yourself skips all of that, and fas doesn't exist in FontAwesome 4, so on an older Bootstrap 3 theme the icon renders as nothing at all.

The good news is you don't have to hardcode anything here, because this template does get parsed:

$genericMail = $tp->parseTemplate($TEMPLATE['ICON_EMAIL'], true, $sc);

So {GLYPH: type=fa-envelope} works in this file and resolves through the proper path. (That is the difference between this and #5425, where the token would have landed in a constant nothing parses.)

A theme override can recurse forever

$EMAILPRINT_TEMPLATE['email'] contains {EMAILICON: url=1}, and render_emailprint() parses that template. {EMAILICON} calls sc_emailicon() (news_shortcodes.php#L732-L741), which calls render_emailprint() again.

With the shipped template it terminates, because the nested call carries url=1 and takes the else if (!empty($parm['url'])) branch. But the guard is only that parm. A theme that overrides the template and writes href='{EMAILICON}', forgetting url=1, re-enters the full branch and recurses until PHP runs out of stack or memory.

That's a sharp edge to hand theme authors. Better to build the URL directly in the template layer, or add an explicit re-entry guard (a static flag, or pass the URL in as $sc data before parsing) so a missing parm degrades instead of exploding.

$parm = ($look == 0) ? null : $parm;

Both core callers pass $look of 1 or 2 (news_shortcodes.php#L740 and #L749), so this line never fires in core and only affects third-party callers that ask for both icons at once. Discarding their parms silently is a surprising thing to do to them. If the reason is that url=1 makes no sense when two URLs are being returned, unset just that key and say so in a comment, rather than dropping everything.

Housekeeping

Every replaced line is commented out rather than deleted, which roughly doubles the size of the diff and makes the new logic hard to follow. The // Probably redundant if using templates. Left here for legacy purposes? notes read like open questions to yourself; if they're genuine questions, worth asking them on the thread so we can answer them, and if they're not, they can go.

Also $EMAILPRINT_TEMPLATE needs if (!isset($EMAILPRINT_TEMPLATE['email'])) style guards, or a theme that overrides the file only partly will find the keys it didn't set are missing rather than falling back to core. getCoreTemplate() with $merge handles that, but only for arrays it can see; worth a check either way.

Swap the hardcoded prefixes for {GLYPH: ...} and close the recursion hole and this is most of the way there.