Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Joomla CMS #27397 #1514

Closed
wants to merge 1 commit into from

4 participants

@nicksavov

This is for CMS tracker id # 27397.

Link to tracker item:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=27397

The bug:
Template style doesn't show up if there's a space in the name tag of templateDetails.xml and the template is installed via the Discover feature

The pull request for the language strings is at:
joomla/joomla-cms#346

@eddieajau

Nick, can you change the title to "Joomla CMS #27397 ", add the problem the patch fixes and also a link to the tracker item. Just makes for a more informative entry in the changelog (and helps we reviews get to the info quickly). Thanks.

@nicksavov

Yes, I updated the comment. Did you want me to add "the problem the patch fixes and also a link to the tracker item" within the title or is it OK within the comment?

@eddieajau

What you've done is perfect, thanks!

@nicksavov

You're welcome! Thanks for helping me out, too :)

Cheers

@realityking realityking commented on the diff
libraries/joomla/installer/adapters/template.php
@@ -488,6 +488,15 @@ public function discover()
continue;
}
$manifest_details = JInstaller::parseXMLInstallFile(JPATH_SITE . "/templates/$template/templateDetails.xml");
+
+ if ($template != $manifest_details['name'])
+ {
+ JError::raiseWarning(100, JText::_('JLIB_INSTALLER_ERROR_TPL_DISCOVER_NAMES_DIFFERENT'));
@realityking Collaborator

We don't use JError in the platform anymore, instead use the JLog workaround. (There are a number of examples for this)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nicksavov

Woah. That's cool that http://developer.joomla.org/pulls/pulls/1514.html does that.

OK, I'll research those things and try and get everything corrected this weekend. Thanks Rouven!

@LouisLandry

@nicksavov thanks for putting this together. I've marked it for the 12.3 milestone (a couple of months out). If you can get the style issues sorted out and the JError usage adjusted we'd love to get this merged into the platform. If you need help with anything feel free to ask here, or on the platform list.

I'm going to close this pull request for now. That doesn't mean it is being rejected, just that you need to work on it a little more. Please re-open it once you've got those things fixed up and it is ready to be reviewed again. Looking forward to getting it merged.

@LouisLandry LouisLandry closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 3, 2012
  1. @nicksavov
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 0 deletions.
  1. +9 −0 libraries/joomla/installer/adapters/template.php
View
9 libraries/joomla/installer/adapters/template.php
@@ -488,6 +488,15 @@ public function discover()
continue;
}
$manifest_details = JInstaller::parseXMLInstallFile(JPATH_SITE . "/templates/$template/templateDetails.xml");
+
+ if ($template != $manifest_details['name'])
+ {
+ JError::raiseWarning(100, JText::_('JLIB_INSTALLER_ERROR_TPL_DISCOVER_NAMES_DIFFERENT'));
@realityking Collaborator

We don't use JError in the platform anymore, instead use the JLog workaround. (There are a number of examples for this)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ continue;
+ // Ignore if directory name is different than the templateDetails.xml <name> tag
+ }
+
$extension = JTable::getInstance('extension');
$extension->set('type', 'template');
$extension->set('client_id', $site_info->id);
Something went wrong with that request. Please try again.