Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression 3.7: last template style preview not working #13637

Closed
joeforjoomla opened this issue Jan 18, 2017 · 9 comments
Closed

Regression 3.7: last template style preview not working #13637

joeforjoomla opened this issue Jan 18, 2017 · 9 comments

Comments

@joeforjoomla
Copy link
Contributor

joeforjoomla commented Jan 18, 2017

Steps to reproduce the issue

Install a new template on Joomla 3.7

Try to open the preview using the URL such as:
http://joomla37/?template=mytemplate

Expected result

The frontend uses the latest installed template named 'mytemplate'

Actual result

The default template is still used

System information (as much as possible)

The code change in site.php included in the function 'getTemplate' caused the regression.

Working code on Joomla 3.6.5:

if (!$templates = $cache->get('templates0' . $tag))
		{
			// Load styles
			$db = JFactory::getDbo();
			$query = $db->getQuery(true)
				->select('id, home, template, s.params')
				->from('#__template_styles as s')
				->where('s.client_id = 0')
				->where('e.enabled = 1')
				->join('LEFT', '#__extensions as e ON e.element=s.template AND e.type=' . $db->quote('template') . ' AND e.client_id=s.client_id');

			$db->setQuery($query);
			$templates = $db->loadObjectList('id');

			foreach ($templates as &$template)
			{
				$registry = new Registry;
				$registry->loadString($template->params);
				$template->params = $registry;

				// Create home element
				if ($template->home == 1 && !isset($templates[0]) || $this->_language_filter && $template->home == $tag)
				{
					$templates[0] = clone $template;
				}
			}

			$cache->store($templates, 'templates0' . $tag);
		}

		if (isset($templates[$id]))
		{
			$template = $templates[$id];
		}
		else
		{
			$template = $templates[0];
		}

Regression code on Joomla 3.7, the 'by reference' variable $template is overwritten in the final $template = $templates[0]; so the latest template style is lost in the $templates array.

if ($cache->contains($cacheId))
		{
			$templates = $cache->get($cacheId);
		}
		else
		{
			// Load styles
			$db = JFactory::getDbo();
			$query = $db->getQuery(true)
				->select('id, home, template, s.params')
				->from('#__template_styles as s')
				->where('s.client_id = 0')
				->where('e.enabled = 1')
				->join('LEFT', '#__extensions as e ON e.element=s.template AND e.type=' . $db->quote('template') . ' AND e.client_id=s.client_id');

			$db->setQuery($query);
			$templates = $db->loadObjectList('id');

			foreach ($templates as &$template)
			{
				// Create home element
				if ($template->home == 1 && !isset($template_home) || $this->_language_filter && $template->home == $tag)
				{
					$template_home = clone $template;
				}

				$template->params = new Registry($template->params);
			}

			// Add home element, after loop to avoid double execution
			if (isset($template_home))
			{
				$template_home->params = new Registry($template_home->params);
				$templates[0] = $template_home;
			}

			$cache->store($templates, $cacheId);
		}

		if (isset($templates[$id]))
		{
			$template = $templates[$id];
		}
		else
		{
			$template = $templates[0];
		}

Additional comments

@joeforjoomla joeforjoomla changed the title Regression 3.7: last template preview not working Regression 3.7: last template style preview not working Jan 18, 2017
@zero-24
Copy link
Contributor

zero-24 commented Jan 18, 2017

Feel free to send a PR that fix it :)

@joeforjoomla
Copy link
Contributor Author

Please open a PR at your end or merge the issue to the 3.7 branch

@zero-24
Copy link
Contributor

zero-24 commented Jan 19, 2017

merge the issue to the 3.7 branch

I'm confused. What should be merged to what? As we are in development of 3.7.0 the staging branch contains the code for 3.7.0 and an issue cant be merged to anything?

@joeforjoomla
Copy link
Contributor Author

I'm confused too. I've just reported a regression issue that have to be fixed.

@mbabker
Copy link
Contributor

mbabker commented Jan 19, 2017

You provided a code solution in your issue and were asked to open a pull request containing your code changes to address the issue.

However also note that section of code purposefully changed in #12688 to address another issue so you will need to validate your proposed changes do not introduce other regressions.

@joeforjoomla
Copy link
Contributor Author

Well that's not the case, i didn't provided a code solution but only posted and compared the current Joomla 3.6 code and the new Joomla 3.7 code that has the issue.

@mbabker
Copy link
Contributor

mbabker commented Jan 19, 2017

Unfortunately just posting a snippet of the 3.6.5 code and saying "this works" doesn't help much, especially since the file's history indicates several changes made in the class (and that method specifically). Isolating a particular change would be helpful because right now there are at least 4 changes to that method alone compared to 3.6.5.

@ghost
Copy link

ghost commented Feb 5, 2017

Can't confirm Issue on latest 3.7-staging.

@mbabker
Copy link
Contributor

mbabker commented Feb 5, 2017

Probably because the pull request for this issue was merged 3 days ago.

@mbabker mbabker closed this as completed Feb 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants