Port all pane settings (contextual links) from administrative interface to IPE

+++ b/panels_ipe/js/panels_ipe_menu.js
@@ -0,0 +1,28 @@
+      var $context = context instanceof Document ? $('body') : $(context);
+
+      $context.once(menu_class, function() {
...
+})(jQuery, 'panels-ipe-menu', 'opened');

+++ b/panels_ipe/panels_ipe.module
@@ -1,172 +1,253 @@
+ * Implements hook_footer().

Why is it better then old good $('.' + menu_class, context)?

+++ b/panels_ipe/js/panels_ipe_menu.js
@@ -0,0 +1,28 @@
+})(jQuery, 'panels-ipe-menu', 'opened');

Why is it better then storing these classes as local string variable with internal definition?

+++ b/panels_ipe/panels_ipe.module
@@ -1,172 +1,253 @@
+ * Implements hook_footer().

Wrong hook comment

+++ b/panels_ipe/panels_ipe.module
@@ -1,172 +1,253 @@
+ * Implements theme_HOOK().

This comment is useless. add info about actual theme name and description. The same for other theme implementations.

+++ b/panels_ipe/panels_ipe.module
@@ -1,172 +1,253 @@
+    'title' => t('Options') . theme_links(array(

use theme('links')

+++ b/panels_ipe/panels_ipe.module
@@ -1,172 +1,253 @@
-  drupal_alter('panels_ipe_pane_links', $vars['links'], $context);

@@ -179,37 +260,29 @@ function template_preprocess_panels_ipe_add_pane_button(&$vars) {
-  drupal_alter('panels_ipe_region_links', $vars['links'], $context);

Don't we really need alter here?

+++ b/plugins/display_renderers/panels_renderer_editor.class.php
@@ -430,18 +453,19 @@ class panels_renderer_editor extends panels_renderer_standard {
+      $info = panels_get_cache(isset($pane->cache['method']) ? $pane->cache['method'] : 0);

Put NULL instead of 0 (see ctools_get_plugins())

I've applied this patch on a fresh panopoly build and here is what I've got:
Strict warning: Declaration of panels_renderer_ipe::render_region() should be compatible with that of panels_renderer_editor::render_region() in require_once() (line 10 of profiles/panopoly/modules/contrib/panels/panels_ipe/plugins/display_renderers/panels_renderer_ipe.class.php).
Strict warning: Declaration of panels_renderer_ipe::render_pane() should be compatible with that of panels_renderer_editor::render_pane() in require_once() (line 10 of profiles/panopoly/modules/contrib/panels/panels_ipe/plugins/display_renderers/panels_renderer_ipe.class.php).
Strict warning: Declaration of panels_renderer_ipe::render_pane_content() should be compatible with that of panels_renderer_standard::render_pane_content() in require_once() (line 10 of panopoly/profiles/panopoly/modules/contrib/panels/panels_ipe/plugins/display_renderers/panels_renderer_ipe.class.php).

Also there is no possibility save page from IPE. See attached screenshot.

Back to theme_links()

Using it directly you're omitting:

1. Preprocess functions
2. Process functions
3. Hook suggestions
4. Possible theme_wrappers
5. Debug support

However, panels_renderer_editor.class.php uses the same approach, so....

#11 works as a charm for me. Good work!

Looks good #11
Let's have it merged to upcoming release

#14 works well.

Check default panels page - style of contextual menu for panes broken.

#18 fixed issue with empty pane actions menu as well. It was bottom links array, that was null for role with poor permissions.

One of the nice pieces of panelizer is the ability to restrict edit abilities to certain roles or users per content type. This patch would open existing sites to potential security issues because new functionality would be given to potentially unprivileged users (who have limited panelizer access)

At a minimum, this feature should be wrapped into opt-in permissions.

Secondly, this patch offers a bunch of code cleanup. While in general I think thats great to clean up code, it potentially breaks other patches while not providing functionality. If we're doing a general code cleanup, that should be its own clean patch (which other issues can be re-rolled against)

Okay, so upon further review -- it looks like permissions are being respected for unprivileged users with IPE access. There was concern around giving group administrators elevated privileges because the contextual menu may expose links that aren't properly checking access. While this wouldn't be a bug in panels, I didn't want this patch to unintentionally expose security issues. Luckily it appears (right now) that its not the case. So this is a moot point.

On the second part, in general we like to split up patches into syntax and features. Panels is at a stable release right now, and many people run the dev version as well as the release. Every patch that makes it into panels shouldn't break head. Thats why we have smaller patches, per issue, so its easier to review.

@BR0kEN: Thanks for making this patch and putting all the effort you've put into it!

Unfortunately, this is really hard to review because of all the unrelated changes. It would take quite a bit of time just to sift through which changes are necessary to implement this functionality and which are just fixing coding style. As the author of the patch, this should be easier for you! Also, the size of the patch makes it difficult to tell if these changes would have collateral damage on something else, which is super important given the level of stability of Panels.

I'd like to second @japerry's call to break reduce this patch to just the changes necessary to implement this functionality, and then start a new issue for the coding style fixes.

Thanks again!

Patch makes a lot of changes I'd love to see, however not all at once. It will for sure break work done in #2462331: IPE insufficient for Panelizer (data loss when using revisions). Then there's issues with code, and it's hard to review because of changes.

+++ b/panels.module
@@ -322,6 +322,10 @@ function panels_permission() {
+    'allow deleting panels' => array(
+      'title' => t('Allow deleting panels'),
+      'description' => t('Allows a user to delete panels.'),
+    ),

+++ b/plugins/display_renderers/panels_renderer_editor.class.php
@@ -454,47 +489,64 @@ class panels_renderer_editor extends panels_renderer_standard {
+    if (user_access('allow deleting panels')) {
+      $links['bottom']['remove'] = array(
+        'title' => t('Remove'),
+        'href' => '#',
+        'attributes' => array(
+          'class' => array('pane-delete'),
+          'id' => "pane-delete-panel-pane-$pane->pid",
+        ),
+      );
+    }

New permission is defined, is called, but never added to a role with relevance?

So now people can't delete items in IPE unless permissions updated? I mean, a change record would fix. But there should be kind of update to add this to users who can already handle this functionality.

+1 for separating off the misc improvements, and I'll be happy to help review it :)

Drupal 7 is no longer supported, closing.