Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 3 additions & 23 deletions administrator/components/com_menus/controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ class MenusController extends JControllerLegacy
/**
* Method to display a view.
*
* @param boolean If true, the view output will be cached
* @param array An array of safe url parameters and their variable types, for valid values see {@link JFilterInput::clean()}.
* @param boolean $cachable If true, the view output will be cached
* @param array|boolean $urlparams An array of safe url parameters and their variable types, for valid values see {@link JFilterInput::clean()}.
*
* @return JController This object to support chaining.
* @return JController This object to support chaining.
* @since 1.5
*/
public function display($cachable = false, $urlparams = false)
Expand All @@ -35,26 +35,6 @@ public function display($cachable = false, $urlparams = false)
$layout = $this->input->get('layout', 'default');
$id = $this->input->getInt('id');

// Check for edit form.
if ($view == 'menu' && $layout == 'edit' && !$this->checkEditId('com_menus.edit.menu', $id)) {

// Somehow the person just went to the form - we don't allow that.
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $id));
$this->setMessage($this->getError(), 'error');
$this->setRedirect(JRoute::_('index.php?option=com_menus&view=menus', false));

return false;
}
elseif ($view == 'item' && $layout == 'edit' && !$this->checkEditId('com_menus.edit.item', $id)) {

// Somehow the person just went to the form - we don't allow that.
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $id));
$this->setMessage($this->getError(), 'error');
$this->setRedirect(JRoute::_('index.php?option=com_menus&view=items', false));

return false;
}

parent::display();

return $this;
Expand Down
10 changes: 0 additions & 10 deletions administrator/components/com_menus/controllers/item.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,6 @@ public function save($key = null, $urlVar = null)
$context = 'com_menus.edit.item';
$recordId = $this->input->getInt('id');

if (!$this->checkEditId($context, $recordId))
{
// Somehow the person just went to the form and saved it - we don't allow that.
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $recordId));
$this->setMessage($this->getError(), 'error');
$this->setRedirect(JRoute::_('index.php?option=com_menus&view=items' . $this->getRedirectToListAppend(), false));

return false;
}

// Populate the row id from the session.
$data['id'] = $recordId;

Expand Down
10 changes: 0 additions & 10 deletions administrator/components/com_menus/controllers/menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ public function save($key = null, $urlVar = null)
$task = $this->getTask();
$recordId = $this->input->getInt('id');

if (!$this->checkEditId($context, $recordId))
{
// Somehow the person just went to the form and saved it - we don't allow that.
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $recordId));
$this->setMessage($this->getError(), 'error');
$this->setRedirect(JRoute::_('index.php?option='.$this->option.'&view='.$this->view_list.$this->getRedirectToListAppend(), false));

return false;
}

// Make sure we are not trying to modify an administrator menu.
if (isset($data['client_id']) && $data['client_id'] == 1){
JError::raiseNotice(0, JText::_('COM_MENUS_MENU_TYPE_NOT_ALLOWED'));
Expand Down
20 changes: 5 additions & 15 deletions administrator/components/com_modules/controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ class ModulesController extends JControllerLegacy
/**
* Method to display a view.
*
* @param boolean If true, the view output will be cached
* @param array An array of safe url parameters and their variable types, for valid values see {@link JFilterInput::clean()}.
* @param boolean $cachable If true, the view output will be cached
* @param array|boolean $urlparams An array of safe url parameters and their variable types, for valid values see {@link JFilterInput::clean()}
.
*
* @return JController This object to support chaining.
* @return JController This object to support chaining.
* @since 1.5
*/
public function display($cachable = false, $urlparams = false)
Expand All @@ -38,17 +39,6 @@ public function display($cachable = false, $urlparams = false)
$layout = $this->input->get('layout', 'default');
$id = $this->input->getInt('id');

// Check for edit form.
if ($view == 'module' && $layout == 'edit' && !$this->checkEditId('com_modules.edit.module', $id))
{
// Somehow the person just went to the form - we don't allow that.
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $id));
$this->setMessage($this->getError(), 'error');
$this->setRedirect(JRoute::_('index.php?option=com_modules&view=modules', false));

return false;
}

parent::display();
return parent::display();
}
}
4 changes: 4 additions & 0 deletions administrator/language/en-GB/en-GB.lib_joomla.ini
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,10 @@ JLIB_HTML_DATE_RELATIVE_MINUTES_0="%s minutes ago"
JLIB_HTML_DATE_RELATIVE_WEEKS="%s weeks ago"
JLIB_HTML_DATE_RELATIVE_WEEKS_1="%s week ago"
JLIB_HTML_DATE_RELATIVE_WEEKS_0="%s weeks ago"
JLIB_HTML_EDIT_MENU_ITEM="Edit menu item"
JLIB_HTML_EDIT_MENU_ITEM_ID="Item id: %s"
JLIB_HTML_EDIT_MODULE="Edit module"
JLIB_HTML_EDIT_MODULE_IN_POSITION="Position: %s"
JLIB_HTML_EDITOR_CANNOT_LOAD="Cannot load the editor"
JLIB_HTML_END="End"
JLIB_HTML_ERROR_FUNCTION_NOT_SUPPORTED="Function not supported."
Expand Down
4 changes: 4 additions & 0 deletions language/en-GB/en-GB.lib_joomla.ini
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,10 @@ JLIB_HTML_DATE_RELATIVE_MINUTES_0="%s minutes ago"
JLIB_HTML_DATE_RELATIVE_WEEKS="%s weeks ago"
JLIB_HTML_DATE_RELATIVE_WEEKS_1="%s week ago"
JLIB_HTML_DATE_RELATIVE_WEEKS_0="%s weeks ago"
JLIB_HTML_EDIT_MENU_ITEM="Edit menu item"
JLIB_HTML_EDIT_MENU_ITEM_ID="Item id: %s"
JLIB_HTML_EDIT_MODULE="Edit module"
JLIB_HTML_EDIT_MODULE_IN_POSITION="Position: %s"
JLIB_HTML_EDITOR_CANNOT_LOAD="Cannot load the editor"
JLIB_HTML_END="End"
JLIB_HTML_ERROR_FUNCTION_NOT_SUPPORTED="Function not supported."
Expand Down
60 changes: 60 additions & 0 deletions layouts/joomla/edit/frontediting_modules.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php
/**
* @package Joomla.Site
* @subpackage Layout
*
* @copyright Copyright (C) 2005 - 2013 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/

defined('_JEXEC') or die;

// JLayout for standard handling of the edit modules:

$moduleHtml =& $displayData['moduleHtml'];
$mod = $displayData['module'];
$position = $displayData['position'];

static $jsNotOut =true;

$app = JFactory::getApplication();

$cannotEditFrontend = $app->isAdmin() || !JFactory::getUser()->authorise('core.manage', 'com_modules');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this check should be done outside the layout

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it could. The idea was to completely "outsource" to the JLayout class that task, but if you prefer we can do it in another "outsourced" way ;-)



if (!$moduleHtml)
{
return;
}

if ($cannotEditFrontend
|| preg_match('/<(?:div|span|nav|ul) [^>]*class="[^"]* jmoddiv"/', $moduleHtml))
{
// Module isn't enclosing in a div with class or handles already module edit button:
return;
}

// Add css class jmoddiv and data attributes for module-editing URL and for the tooltip:
$editUrl = JURI::base() . 'administrator/' . 'index.php?option=com_modules&view=module&layout=edit&id=' . (int) $mod->id;

// Add class, editing URL and tooltip, and if module of type menu, also the tooltip for editing the menu item:
$moduleHtml = preg_replace('/^(<(?:div|span|nav|ul) [^>]*class="[^"]*)"/',
'\\1 jmoddiv" data-jmodediturl="' . $editUrl. '"'
. ' data-jmodtip="'
. JHtml::tooltipText(JText::_('JLIB_HTML_EDIT_MODULE'), htmlspecialchars($mod->title) . '<br />' . sprintf(JText::_('JLIB_HTML_EDIT_MODULE_IN_POSITION'), htmlspecialchars($position)), 0)
. '"'
. ($mod->module == 'mod_menu' ? '" data-jmenuedittip="' . JHtml::tooltipText('JLIB_HTML_EDIT_MENU_ITEM', 'JLIB_HTML_EDIT_MENU_ITEM_ID') . '"' : ''),
$moduleHtml);

if ($jsNotOut)
{
// Load once booststrap tooltip and add stylesheet and javascript to head:
$jsNotOut = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for? JHtml already caches assets

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching early instead of late code that does not need to be executed most of the time is good CS practice. I don't see your issue here, sorry.

JHtml::_('bootstrap.tooltip');
JHtml::_('bootstrap.popover');

JFactory::getDocument()->addStyleSheet('media/system/css/frontediting.css')
->addScript('media/system/js/frontediting.js');
}

?>
6 changes: 5 additions & 1 deletion libraries/joomla/document/html/renderer/modules.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ public function render($position, $params = array(), $content = null)

foreach (JModuleHelper::getModules($position) as $mod)
{
$buffer .= $renderer->render($mod, $params, $content);
$moduleHtml = trim($renderer->render($mod, $params, $content));

JLayoutHelper::render('joomla.edit.frontediting_modules', array('moduleHtml' => &$moduleHtml, 'module' => $mod, 'position' => $position));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a good place for the permission check. Something like:

if (JFactory::getApplication()->isAdmin() || JFactory::getUser()->authorise('core.manage', 'com_modules'))
{
    JLayoutHelper::render('joomla.edit.frontediting_modules', array('moduleHtml' => &$moduleHtml, 'module' => $mod, 'position' => $position));
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the layout already returns HTML so why add it as param? Wouldn't be the same:

$buffer .= JLayoutHelper::render('joomla.edit.frontediting_modules', array('module' => $mod, 'position' => $position));

Doing the enclosing div check outside the layout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a good place for the permission check. Something like:
yes, it is.

Also the layout already returns HTML so why add it as param? Wouldn't be the same: ...

Not really, as the JLayout does adjust the passed HTML (and as it's a JLayout, other templates can have their own override for their own markup if needed).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core.manage is the incorrect permission here. It should be core.edit at miniumum if not also core.edit.state.


$buffer .= $moduleHtml;
}
return $buffer;
}
Expand Down
33 changes: 0 additions & 33 deletions libraries/legacy/controller/form.php
Original file line number Diff line number Diff line change
Expand Up @@ -311,23 +311,6 @@ public function cancel($key = null)
// Attempt to check-in the current record.
if ($recordId)
{
// Check we are holding the id in the edit list.
if (!$this->checkEditId($context, $recordId))
{
// Somehow the person just went to the form - we don't allow that.
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $recordId));
$this->setMessage($this->getError(), 'error');

$this->setRedirect(
JRoute::_(
'index.php?option=' . $this->option . '&view=' . $this->view_list
. $this->getRedirectToListAppend(), false
)
);

return false;
}

if ($checkin)
{
if ($model->checkin($recordId) === false)
Expand Down Expand Up @@ -650,22 +633,6 @@ public function save($key = null, $urlVar = null)

$recordId = $this->input->getInt($urlVar);

if (!$this->checkEditId($context, $recordId))
{
// Somehow the person just went to the form and tried to save it. We don't allow that.
$this->setError(JText::sprintf('JLIB_APPLICATION_ERROR_UNHELD_ID', $recordId));
$this->setMessage($this->getError(), 'error');

$this->setRedirect(
JRoute::_(
'index.php?option=' . $this->option . '&view=' . $this->view_list
. $this->getRedirectToListAppend(), false
)
);

return false;
}

// Populate the row id from the session.
$data[$key] = $recordId;

Expand Down
33 changes: 33 additions & 0 deletions media/system/css/frontediting.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* @copyright Copyright (C) 2005 - 2013 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/

/* Module edit in front-end */

.jmoddiv.jmodinside {
position: relative;
top: 0;
left: 0;
}
.btn.jmodedit
{
z-index: 1001;
display: block;
position: absolute;
top: 0;
right: 0;
}
html[dir=rtl] .btn.jmodedit
{
right: auto;
left: 0;
}

/* Menu edit in front-end */

.btn.jfedit-menu
{
z-index: 1002;
display: block;
}
Loading