The WYSIWYG module has a new release that incorporates 4 of the patches Panopoly is using, see Release Notes. According to that page, the released version doesn't support PHP 5.3 anymore because of a change in array syntax. If Panopoly still needs to support PHP 5.3, then there is a commit in the WYSIWYG module dev branch that uses the old array syntax.

Comments

cboyden created an issue. See original summary.

cboyden’s picture

Assigned: cboyden » Unassigned
Status: Active » Needs review
StatusFileSize
new1.28 KB

Patch is attached. If PHP 5.3 support is necessary I can update it to include that.

dsnopek’s picture

dsnopek’s picture

Manual testing looked good!

I've got mixed feelings on PHP 5.3 support. Our tests haven't been running on PHP 5.3 for a while anyway. And I don't know that we really want to commit to patching all contrib modules for PHP 5.3 going forward for a version of PHP that was EOL'd over 5 years ago. I'm going to commit this and we'll see if any PHP 5.3 fans come out of the woodwork. :-)

  • dsnopek committed 8a921e7 on 7.x-1.x authored by cboyden
    Issue #3094890 by cboyden: Update WYSIWYG to version 2.6
    
dsnopek’s picture

Status: Needs review » Fixed

Committed!

  • dsnopek committed a28e7f7 on 7.x-1.x
    Revert "Issue #3094890 by cboyden: Update WYSIWYG to version 2.6"
    
    This...
dsnopek’s picture

Status: Fixed » Needs work

After merging, this appears to break our tests:

https://travis-ci.org/panopoly/panopoly/builds/634477995

I'm not sure why my original test run on Travis came back green... I'm going to look into that.

Here's a new test running with this change reverted:

https://travis-ci.org/panopoly/panopoly/builds/634775305

If that passes, then we know we need to fix this one up.

dsnopek’s picture

Yeah, the tests with the change reverted seem to be passing.

I think the reason the tests didn't catch it the first time is because I keep forgetting to run 'robo build:drupal-org-make' to accommodate the monorepo, so I added a git pre-commit hook to the RoboFile to try and prevent me from committing without running that. :-)

Anyway, the test failure is just an alert appearing that the tests don't expect:

  ╳  unexpected alert open: {Alert text : Are you sure you want to change the text format?
  ╳  
  ╳  Changing the text format to HTML and enabling the associated editor will permanently remove content that is not allowed in both text formats.
  ╳  
  ╳  Cancel and save your changes before switching the text format to avoid losing data.}
cboyden’s picture

Status: Needs work » Needs review
StatusFileSize
new1.88 KB

Here's a patch that adds some steps that confirm or cancel a browser popup, and then adds them to the live preview WYSIWYG test.

dsnopek’s picture

Here's a Travis build that will hopefully be accurate this time:

https://travis-ci.org/panopoly/panopoly/jobs/634925485

I'm gonna try running this locally too.

dsnopek’s picture

StatusFileSize
new2.46 KB

Manual testing worked again (I wanted to redo it, just in case I was using the wrong versions locally).

Tests on Travis are failing, though, with:

When I select "HTML" from "Editor"                                                                   # Drupal\DrupalExtension\Context\MinkContext::selectOption()
    And I confirm the popup                                                                              # TestSubContext::iConfirmPopup()
      no alert open

Could be a timing thing? The test is passing for me locally.

Here's a patch that adds a short wait.

EDIT: Here's a new Travis build: https://travis-ci.org/panopoly/panopoly/builds/634948363

dsnopek’s picture

StatusFileSize
new3.04 KB

Alright, that didn't work! Let's try this.

EDIT: Here's a new Travis build https://travis-ci.org/panopoly/panopoly/builds/635457251

  • dsnopek committed 3126b05 on 7.x-1.x authored by cboyden
    Issue #3094890 by dsnopek, cboyden: Update WYSIWYG to version 2.6
    
dsnopek’s picture

Status: Needs review » Fixed

Well, that seemed to work! Committing :-)

Status: Fixed » Closed (fixed)

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