Problem/Motivation

The Manual Crop module uses inline Javascript. Any site using a strong Content Security Policy will want to exclude inline Javascript, and the Manual Crop button will break. Existing Panopoly Behat tests only check for the presence of the Crop button, not whether it works.

Steps to reproduce

  1. Install the latest dev version of Panopoly 7.x with starter content
  2. Set a Content Security Policy header that disallows inline JS (do not output the 'unsafe-inline' keyword).
  3. Customize a page and create an Image widget. Note that the Crop button appears.
  4. Click the Crop button.
  5. Note that nothing happens.

Proposed resolution

Patch Manual Crop (see #3177209: Remove inline Javascript so that CSP doesn't require 'unsafe-inline') and add a Behat test that verifies what we can verify - probably we can test that the button opens the Crop dialog, the expected Crop buttons are present, and saving the crop returns us to the host page.

Remaining tasks

Write a Behat test that demonstrates the issue - DONE.
Update the Panopoly Images makefile to apply Manual Crop patch - DONE.
Run Behat tests to verify the fix - DONE.
Manual testing to verify that Crop works as expected - DONE.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#5 panopoly_images-manualcrop-csp-3177807-5.patch468 bytescboyden
#3 panopoly_test-manualcrop-csp-3177807-3.patch3.02 KBcboyden
#2 panopoly_test-manualcrop-csp-3177807-2.patch3.09 KBcboyden

Comments

cboyden created an issue. See original summary.

cboyden’s picture

cboyden commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new panopoly_test-manualcrop-csp-3177807-2.patch3.09 KB

Here's a patch that adds a Behat test to verify the basic functions of Manual Crop: Activate the crop button; make a selection (using Maximize selection); save the crop; reopen and undo the crop (using Remove selection). MC does something to duplicate and then hide its buttons, so the patch required adding a new Behat region to find the active links.

The patch in the linked issue #3177209: Remove inline Javascript so that CSP doesn't require 'unsafe-inline' needs to be updated to do some button text switching; until that's done, this test will not pass with the patch in that issue.

cboyden’s picture

cboyden commented
StatusFileSize
new panopoly_test-manualcrop-csp-3177807-3.patch3.02 KB

Updated patch to reflect that Remove selection is available from the start on a new, uncropped upload.

cboyden’s picture

cboyden commented

The new Manual Crop test is passing, see https://travis-ci.org/github/cboyden/panopoly/builds/737480633 for results. Failures on some of the update jobs seem to be unrelated to the new test. This is without a strict CSP; it just verifies that manual crop works without the new patch and without CSP.

I've tested locally with a strict CSP and the test fails, so this verifies that the test detects what it's supposed to detect. Once the patch in #3177209: Remove inline Javascript so that CSP doesn't require 'unsafe-inline' is updated, if this test passes, we're good.

cboyden’s picture

cboyden commented
StatusFileSize
new panopoly_images-manualcrop-csp-3177807-5.patch468 bytes

The manualcrop patch has been updated and passes in my local testing with strict CSP. Here's a patch to add that to the panopoly_images makefile.

Edit: Tests are generally passing: https://travis-ci.org/github/cboyden/panopoly/builds/738447028

cboyden’s picture

cboyden commented
Issue summary: View changes
cboyden’s picture

cboyden commented
Assigned: cboyden » Unassigned

  • dsnopek committed a22d505 on 7.x-1.x authored by cboyden 
    Issue #3177807 by cboyden: Manual Crop doesn't work with strong Content...
dsnopek’s picture

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

Thanks! Committed :-)

Status: Fixed » Closed (fixed)

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