e107 icon indicating copy to clipboard operation
e107 copied to clipboard

Navigation icons uploaded with Media Manager used on custom pages are unexpectedly 350×350px

Open Deltik opened this issue 2 years ago • 6 comments

Bug Description

Due to the reuse of state in e107::getParser(), navigation bar icons that were uploaded to Media Manager will use the same thumbnail dimensions as the ones on cpages.

Using e107 v2.3.2, this is the stack where the thumbnail dimensions are specified:

page_template.php:98, include_once()
e107_class.php:3603, e107::_getTemplate()
e107_class.php:3318, e107::getCoreTemplate()
page.php:657, pageClass->processViewPage()
page.php:71, include()
index.php:107, {main}()

The thumbnail dimensions are set in this stack:

e_parse_class.php:2259, e_parse->thumbCrop()
setimage.php:14, setimage_shortcode()
shortcode_handler.php:1312, e_parse_shortcode->doCode()
shortcode_handler.php:1020, preg_replace_callback()
shortcode_handler.php:1020, e_parse_shortcode->parseCodes()
e_parse_class.php:848, e_parse->parseTemplate()
form_handler.php:4789, e_form->renderRelated()
page_shortcodes.php:760, cpage_shortcodes->sc_cpagerelated()
shortcode_handler.php:1154, e_parse_shortcode->doCode()
shortcode_handler.php:1020, preg_replace_callback()
shortcode_handler.php:1020, e_parse_shortcode->parseCodes()
e_parse_class.php:848, e_parse->parseTemplate()
page.php:933, pageClass->renderPage()
page.php:860, pageClass->setPage()
page.php:72, include()
index.php:107, {main}()

And navigation icons uploaded to e_MEDIA_IMAGE reuse the dimensions here:

e_parse_class.php:4417, e_parse->toIcon()
navigation_shortcodes.php:266, navigation_shortcodes->sc_nav_link_icon()
shortcode_handler.php:1154, e_parse_shortcode->doCode()
shortcode_handler.php:1020, preg_replace_callback()
shortcode_handler.php:1020, e_parse_shortcode->parseCodes()
e_parse_class.php:848, e_parse->parseTemplate()
navigation_shortcodes.php:330, navigation_shortcodes->sc_nav_link_sub()
shortcode_handler.php:1154, e_parse_shortcode->doCode()
shortcode_handler.php:1020, preg_replace_callback()
shortcode_handler.php:1020, e_parse_shortcode->parseCodes()
e_parse_class.php:848, e_parse->parseTemplate()
sitelinks_class.php:1672, e_navigation->render()
navigation.php:52, navigation_shortcode()
shortcode_handler.php:1312, e_parse_shortcode->doCode()
shortcode_handler.php:1020, preg_replace_callback()
shortcode_handler.php:1020, e_parse_shortcode->parseCodes()
e_parse_class.php:848, e_parse->parseTemplate()
e107_class.php:383, e107::renderLayout()
header_default.php:823, require_once()
page.php:80, include()
index.php:107, {main}()

How to Reproduce

Steps to reproduce the behavior:

  1. Upload a small image to Media Manager.
  2. Use it as a navigation item icon.
  3. Navigate to the custom page.
  4. Observe that the icon is too big.

Expected Behavior

image

Actual Behavior

image

Deltik avatar Aug 03 '23 14:08 Deltik

This patch fixes the symptom of this bug, but I'm concerned about potential side effects:

diff --git a/e107_core/shortcodes/single/navigation.php b/e107_core/shortcodes/single/navigation.php
--- a/e107_core/shortcodes/single/navigation.php	(revision a6e1c0b897a3bf3f3e7bd6ea0382e2556e456379)
+++ b/e107_core/shortcodes/single/navigation.php	(date 1691073309731)
@@ -47,6 +47,8 @@
 	$template		= e107::getCoreTemplate('navigation', $tmpl);	
 	$data 			= $nav->initData($category, $parm);
 
+	e107::getScParser()->parseCodes("{SETIMAGE: default}");
+
 	return $nav->render($data, $template, $parm);
 
 }

Deltik avatar Aug 03 '23 14:08 Deltik

Maybe related: https://github.com/e107inc/e107/issues/4846

Jimmi08 avatar Aug 03 '23 16:08 Jimmi08

Maybe related: https://github.com/e107inc/e107/issues/4143 See answer.

Jimmi08 avatar Aug 03 '23 16:08 Jimmi08

A similar answer here: https://github.com/e107inc/e107/issues/2676

Jimmi08 avatar Aug 03 '23 16:08 Jimmi08

This patch fixes the symptom of this bug, but I'm concerned about potential side effects:

diff --git a/e107_core/shortcodes/single/navigation.php b/e107_core/shortcodes/single/navigation.php
--- a/e107_core/shortcodes/single/navigation.php	(revision a6e1c0b897a3bf3f3e7bd6ea0382e2556e456379)
+++ b/e107_core/shortcodes/single/navigation.php	(date 1691073309731)
@@ -47,6 +47,8 @@
 	$template		= e107::getCoreTemplate('navigation', $tmpl);	
 	$data 			= $nav->initData($category, $parm);
 
+	e107::getScParser()->parseCodes("{SETIMAGE: default}");
+
 	return $nav->render($data, $template, $parm);
 
 }

Perhaps adding {SETIMAGE: default} to the core navigation template is a better idea, at least it would provide theme developers with a way to override this change.

CaMer0n avatar Sep 08 '23 21:09 CaMer0n

The issue is the reuse of the e_parse state for a different scope (first in "navigation", then in "page"). The ideal fix would be to initialize the state for that particular scope rather than share one across the entire execution. Otherwise, we're forcing themes to work around quirks/bugs from the e107 core.

Deltik avatar Sep 11 '23 07:09 Deltik