The change in #1792738: Allow custom file view modes for WYSIWYG display moves the Media WYSIWYG View Mode functionality into the Media WYSIWYG module and refactors the way the module's configuration is stored.

This update doesn't retain any existing configuration and also doesn't show a message to the user to tell them they need to reconfigure it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rooby created an issue. See original summary.

rooby’s picture

Around 20000 users have already done this update, however there are a lot that haven't and might be using this functionality so it still might be beneficial to address this.

joseph.olstad’s picture

Hi rooby , think you can figure out a patch between the two duplicate issues:

#2550431: View mode configuration should be stored along with the file type object, instead of variables

and

#1792738: Allow custom file view modes for WYSIWYG display
?

You had commented that 2550431 contained a patch that would have copied the settings over correctly, (however its patch didn't pass testing) could you maybe piece together the work that was done in that to make a new patch to fix this for those that haven't yet upgraded? I'd propose editing the same hook update , as those that have already upgraded probably manually restored their configuration and their old conf would have been deleted.

rooby’s picture

Status: Active » Needs review
FileSize
1.68 KB

I think the tests failed in that other issue because the tests needed changing for the new storage structure but hadn't been yet.

Here is a quick patch for the config migration. I have done one successful update from beta7 to latest dev with it.

  • joseph.olstad committed 0e035bf on 7.x-2.x authored by rooby
    Issue #2841742 by rooby: Media WYSIWYG View Mode configuration lost...
joseph.olstad’s picture

Status: Needs review » Fixed

Nice work rooby, I'll tag this for rc3

Status: Fixed » Closed (fixed)

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

joseph.olstad’s picture

Status: Closed (fixed) » Needs work

This change caused a regression:

After updating from wetkit 4.14 to 4.15 we noticed that when we add images in the WYSIWYG they get set with a size of 100 x 100 which make them look like thumbnails.

Steps to recreate:
- Create a new wetkit 4.14 site
- Update the site to wetkit 4.15
- Edit one of the existing nodes and in the WYSWIG click the media button and add a file.

Things I tried:
- reverting the wetkit_wysiwyg module and still had the problem.
- Doing a clean wetkit 4.15 install (no update) doesn't have the problem.

When our distro upgraded from media version X to 2.0-rc3 I cloned the module from git and the issue appears when I apply this commit: http://drupalcode.org/media/commit/?id=31e20ae

If I go to the previous commit, the image gets added with no width and height.

It looks like this hook_update_n is not working properly. @nathan is going to show the exact reproduction case momentarily.

http://cgit.drupalcode.org/media/commit/?id=0e035bfd6aba4190be79df55d9f4...

Once we truncated media_view_mode_wysiwyg where everything was hard coded to wysiwyg everything worked.

sylus’s picture

Status: Needs work » Fixed

Think we can keep this as fixed and just focus on related issue. If inappropriate to do so please set back.

natew’s picture

Status: Fixed » Closed (fixed)
AbramOmovich’s picture

Hi, rooby, I've recently performed update from 7.x-2.0-alpha4 to 7.x-2.16 and I noticed that it has mistakes in migrating old variables in media_wysiwyg_update_7205.

AbramOmovich’s picture

joseph.olstad’s picture

Status: Closed (fixed) » Reviewed & tested by the community

I recall this issue,
Thanks for the follow up. Push this improvement in asap.
There's still ~50000 installs that haven't yet upgraded from 1.x to 2.x

This patch should be committed to 2.x and cherry picked for 3.x and 4.x

joseph.olstad’s picture

Status: Reviewed & tested by the community » Needs review

Actually still reviewing it

joseph.olstad’s picture

Abram, I am a bit nervous about making this change, can you describe the symptoms that led you to creating this patch?

Meanwhile I will try to have a look at the various tagged releases and see what possible variations there are in the variable names.

Thanks for your feedback.

joseph.olstad’s picture

Status: Needs review » Fixed

Abram, your patch looks good, thanks!

Status: Fixed » Closed (fixed)

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

joseph.olstad’s picture

this recent change may have caused a regression for those upgrading from 7.x-1.x

still to be confirmed.