Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Using the latest Panelizer, and saving a new revision, data can be lost on subsequent saves.
Steps to reproduce (taken from this Panopoly issue: #2786553: Data loss: Saving in IPE without changes can cause previous change to be lost):
- Install and enable Panelizer 3.3 and Panels 3.6 and the Panels IPE
- Create a new content type, panelize the "Full page override" and use the IPE on the Panelizer default
- Create a new node of this content type
- Click Customize this page.
- Add a new "Custom content" pane to any region
- Click Save in the CTools modal.
- Check the box to create a new revision. Add the revision log message "Added text widget."
- Click Save at the bottom of the page.
- Note your new pane is displayed.
- Click Customize this page again.
- Click Save at the bottom of the page without making any changes.
- Note your new pane is still displayed.
- Refresh the page.
- Note your new pane is no longer displayed.
- Go to the Revisions tab.
- Note that the current revision is the one with the message "Added text widget."
Because this issue is in Javascript, it wasn't caught by the Panelizer tests. But since this can be caused using a basic feature of the last two versions of Panelizer, and it involves losing data, this is a pretty critical issue.
Original summary
IPE leverages links with the cache key incorporated into them in order to allow they currently edited panels to save to the same cache, however some approaches, like panelizer, swap that cache key out from under IPE. IPE does a fair job of keeping up with this despite the fact that it wasn't designed to do this, however the associated "Customize" and "Layout" links in the bottom bar will be out of date with the current cache key after a save event is fired on a panelized node IF a new revision is generated as part of the panelizer save.
This bug prevents any attempts to save the same page twice. If you were to customize, add a block, save a new revisions and then customize and add a block again, you'll end up with a visual block that has no controls and doesn't save to the expected location.
Looking at the associated JS, the #panels-ipe-control-container div is processed at the same time as our new display is rendered to the page. This doesn't work properly because we need to also replace the control container div with new buttons. I attempted:
diff --git a/panels_ipe/plugins/display_renderers/panels_renderer_ipe.class.php b/panels_ipe/plugins/display_renderers/panels_renderer_ipe.class.php
index c959a6d..a814225 100644
--- a/panels_ipe/plugins/display_renderers/panels_renderer_ipe.class.php
+++ b/panels_ipe/plugins/display_renderers/panels_renderer_ipe.class.php
@@ -284,6 +284,9 @@ class panels_renderer_ipe extends panels_renderer_editor {
// rendered.
$this->meta_location = 'inline';
$this->commands[] = ajax_command_replace("#panels-ipe-display-{$this->clean_key}", panels_render_display($this->display, $this));
+ $buttons = &drupal_static('panels_ipe_toolbar_buttons', array());
+ $output = theme('panels_ipe_toolbar', array('buttons' => $buttons));
+ $this->commands[] = ajax_command_replace('#panels-ipe-control-container', $output);
}
else {
// Cancelled. Clear the cache.
But this makes the buttons disappear and the JS doesn't come back. Attempts to edit the JS on my part have all failed. Direct manipulation of the DOM via Chrome/Firebug results in subsequent edits being able to be performed, but the expected save/cancel buttons never display once adding panes.
| Comment | File | Size | Author |
|---|---|---|---|
| #34 | panels-ipe-insufficient-2462331-33.patch | 785 bytes | chrisgross |
| #31 | panels-ipe-insufficient-IE8-2462331-31.patch | 622 bytes | Zemelia |
| #21 | interdiff.txt | 859 bytes | dsnopek |
| #21 | panels-ipe-insufficient-2462331-21.patch | 3.14 KB | dsnopek |
Comments
Comment #1
EclipseGc CreditAttribution: EclipseGc at Acquia commentedComment #2
mglamanSo I was giving this a shot. The below diff fixes some issues with a stale jQuery instance in the IPE's variables.
EDIT: Not sure why "Save" and "Cancel" are missing, exactly. Looks to be a stale selector as well.
Comment #3
mglamanSo this issue all along was just stale jQuery objects in variables. Here is patch with diff from summary and the variable refreshes on the ending of the IPE.
Comment #4
EclipseGc CreditAttribution: EclipseGc at Acquia commentedOk, further debugging against panelizer and last patch was ALMOST there. I think this should do it.
Eclipse
Comment #5
EclipseGc CreditAttribution: EclipseGc at Acquia commentedRerolled and added layout support since I missed that first time through.
Eclipse
Comment #7
EclipseGc CreditAttribution: EclipseGc at Acquia commentedSorry, was working against an old checkout.
Eclipse
Comment #8
mglamanShouldn't we make this a class method? Just invoke method to add commands. In case that static or something changes.
Comment #9
merlinofchaos CreditAttribution: merlinofchaos commentedSO technically the IPE toolbar was implemented with the idea of being extensible, so things outside of panelizer might be able to add buttons. Making it a method would only be useful in-so-much as it would abstract the code a bit, but would otherwise be the same. I'm not sure it's terribly worthwhile.
Comment #10
davidsheart02 CreditAttribution: davidsheart02 commentedI was experiencing the same customize > add pane > save > customize > add pane > see nothing issue as laid out in the issue description. I applied the patch from #7 against my 7.x-3.3+41-dev version and it resolved the issue.
Comment #11
mario_prkos CreditAttribution: mario_prkos commentedI had the same issue. I applied patch #7 to 3.5 version and works for me.
Comment #12
joelstein CreditAttribution: joelstein commentedJust a thought, but there's work over in #2363275: IPE needs to allow for more nuanced save commands which adds hook_panels_ipe_ajax_commands_alter(), and might be usable by Panelizer to resolve this issue?
Comment #13
joelstein CreditAttribution: joelstein commentedComment #14
mglamanjoelstein, possibly that issue could help. However one of the biggest issues is the stale jQuery object. This patches causes the selectors to refresh on change.
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous commentedWe've been using this patch for 6+ months with no problems.
Comment #16
dan.munn CreditAttribution: dan.munn at Unipro Ltd commented+1 Without this path we were noticing that there was an issue with revisions in panelizer with IPE (second save on same page ended up reverting most changes). After applying this patch we're not seeing the same, I will assume due to lack of form re-draw it was editing the same starting revision (not tested) - either way this patch fixes the issue.
Comment #17
cboyden CreditAttribution: cboyden commentedRe-rolled to apply cleanly to latest release - there were enough differences to throw off drush make. The only changes are in the line numbers affected.
Comment #18
DamienMcKennaComment #19
dsnopekComment #20
dsnopekJust a note that I'm able to reproduce this, and the patch solves the problem in my testing! We're going to use this in the next version of Panopoly.
Comment #21
dsnopekHere's a slight improvement to the patch.
Basically, every time we save a new revision in Panelizer, it ends up creating a new IPE editor for the new cache key. If someone is using the IPE for a while, this means we accumulate a whole bunch of editors that aren't actually being used any more. The most noticeable effect of this, is that you'll get the "Unsaved changes" pop-up for old editors, even though you've definitely saved the changes.
My addition to the patch just cleans up the old editors when they're no longer in the list of cache keys.
Comment #22
japerryI dunno, I sorta like my test data disappearing!
The reproduction steps worked, and patch worked! committed.
Comment #24
mudjunky CreditAttribution: mudjunky commentedUnfortunately this commit to Panels 3.7 broke my IPE. It was working using Panels 3.6. Commenting out "// Remove any old editors." code seems to fix the error.
panels_ipe.js?oce6h5:31 Uncaught TypeError: Cannot read property 'indexOf' of undefined
Steps to repro:
1) Customize this page
2) Edit a custom content pane
3) Click "Finish", Nothing happens
4) Click X to close pane editor
5) Save page, changes do stick..
(Also running Chaos Tools 1.10 and Drupal 7.50.)
Thanks :)
Comment #25
dsnopek@mudjunky: Can you confirm the version of CTools you're using? If you're on 1.10, can you try 1.9? I haven't been able to reproduce this on Panopoly which uses CTools 1.9 (because 1.10 has other problems).
Comment #26
dan.munn CreditAttribution: dan.munn at Unipro Ltd commentedmudjunky I think this is a separate point - I raised a bug (and a patch) a long time ago re this - there is a for loop used on an array - in this case you've got some JS that is adding prototype items which will end up in the for loop - see https://www.drupal.org/node/2161223.
Comment #27
mudjunky CreditAttribution: mudjunky commentedHey guys, I appreciate you jumping in.
I am using CTools 1.10 (edited post to reflect). @dsnopek I reverted back to 1.9 and still get same error. Am not using Panopoly though..
I'll keep digging, but any help appreciated.
Comment #28
mudjunky CreditAttribution: mudjunky commented@dan.munn, I applied that patch, but still have same issue. I thought I had it but no go. Will keep at it. Thanks again.
Comment #29
Zemelia CreditAttribution: Zemelia commentedHi, we have issue in IE8.
It doesn't support indexOf for objects:
if (Drupal.settings.PanelsIPECacheKeys.indexOf(i) === -1)Patch attached.
Comment #30
Zemelia CreditAttribution: Zemelia commentedHi, sorry previous one was wrong, correct attached.
Comment #31
Zemelia CreditAttribution: Zemelia commentedOops, again wrong, should be ok at last
Comment #32
chrisgross CreditAttribution: chrisgross commentedI can confirm @mudjunky's problem. The code that removes old editors breaks the IPE and requires me to reload the page to make any further changes. It also allows me to continue editing panes even though I can't save the display, though those changes do not actually get saved to the panes. I had no such problems before that code was added. Here is a patch that removes that code for now, since I don't know enough about the functionality to improve upon it.
Comment #33
chrisgross CreditAttribution: chrisgross commentedComment #34
chrisgross CreditAttribution: chrisgross commentedWhoops, last one has @Zemelia's code in it still. Here's one that should work against the latest unpatched dev.
Comment #35
DamienMcKennaComment #36
chrisgross CreditAttribution: chrisgross commentedIs anyone still successfully using some combination of patches for drafts/moderation of panelizer pages on their D7 sites? I am still having data loss in certain situations and cannot figure out where things are going wrong. There are too many patches involved at this point, I think. This is what I have in my make file:
Comment #37
DamienMcKennaCould folks please test the patch above with the latest -dev snapshots of panelizer and workbench_moderation? Thanks.