If you have a style plugin that has a field with '#type' => 'managed_file', panopoly_magic will break it and make it impossible to upload a file - the error given is:

An unrecoverable error occurred. The uploaded file likely exceeded the maximum file size (2 MB) that this server supports.

This is caused by overzealously moving some internal form elements (like 'form_build_id') under the 'General Settings' fieldset.

By the same token, it also moves the 'Cancel' button under the 'General Settings' fieldset, which looks wierd:

And there are some PHP notices when editing region styles (as opposed to pane styles).

I'll upload a patch to fix soon, but it needs tests before we can commit!

CommentFileSizeAuthor
#12 panopoly_test-style-form-2477469-12.patch4.4 KBdsnopek
#10 style_settings_form-2477469-10.patch4.4 KBmglaman
#9 style_settings_form-2477469-9.patch4.33 KBmglaman
#7 panopoly_test-style-form-2477469-7.patch3.16 KBdsnopek
#6 panopoly_tests-style_settings_form-2477469-6.patch3.16 KBmglaman
#2 panopoly_magic-style-form-2477269-2.patch3.6 KBdsnopek
#1 panopoly_magic-style-form-2477269-1.patch1.65 KBdsnopek
Selection_060.png34.99 KBdsnopek
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Status: Active » Needs review
FileSize
panopoly_magic-style-form-2477269-1.patch1.65 KB

Patch is attached!

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
FileSize
panopoly_magic-style-form-2477269-2.patch3.6 KB

Here is a more comprehensive patch that eliminates a bunch of PHP warnings when editing a Region style (which doesn't have a Pane available to preview).

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Title: Style settings form breaks 'managed_file' elements and puts 'Cancel' button in 'General Settings' fieldset » Style settings form breaks 'managed_file' elements, puts 'Cancel' button in 'General Settings' fieldset, and gives PHP notices on region style settings
Issue summary: View changes
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Issue tags: +Needs tests

This needs tests before we can commit: adding tag!

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Gaggle.net, Inc. commented
Assigned: Unassigned » mglaman

Working on the tests now, patch fixes issue.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Gaggle.net, Inc. commented
FileSize
panopoly_tests-style_settings_form-2477469-6.patch3.16 KB

Here is the test for the working patch in #2

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
FileSize
panopoly_test-style-form-2477469-7.patch3.16 KB

This is the same patch from #6 except renamed to 'panopoly_test' (with no 's') so my Travis script will catch it.

EDIT: Here's the Travis build: https://travis-ci.org/panopoly/panopoly/builds/62756906

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Status: Needs review » Needs work

The tests pass and the Behat feature looks good!

Two points of quick review:

  1. Let's also check that there is a "Cancel" button on region style settings form
  2. And, let's also make sure that the style settings form works on the pane style too
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Gaggle.net, Inc. commented
Assigned: mglaman » Unassigned
FileSize
style_settings_form-2477469-9.patch4.33 KB

Ok! So here's the pane test, which this patch is in fact breaking. Clicking "next" to visit the pane style closes out the modal.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Gaggle.net, Inc. commented
Status: Needs work » Needs review
FileSize
style_settings_form-2477469-10.patch4.4 KB

:) Test failed because test style did not have pane settings callback, only region.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

Trying the tests on Travis one last time: https://travis-ci.org/panopoly/panopoly/builds/62778272

If this passes, I think it's ready to commit!

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
FileSize
panopoly_test-style-form-2477469-12.patch4.4 KB

Here's the same patch from #10, but renamed to start with panopoly_test so that my script for running the tests on Travis will work.

EDIT: Here is the Travis-CI build: https://travis-ci.org/panopoly/panopoly/builds/63023303

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented
Status: Needs review » Fixed

Thanks, @mglaman, for the tests! Committed. :-)

  • dsnopek committed 075fc7e on 7.x-1.x 
    Update Panopoly Magic and Test for Issue #2477469 by dsnopek, mglaman:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.