Hoping to upgrade to use the big improvements achieved in #2126755: Improve Media's WYSIWYG Macro handling

Tried upgrading from media 7.x-2.0-alpha2 to the latest dev version. I upgraded both file entity and media, ran update.php and flushed all caches. also flushed the browser caches.

Still, when I edit a node and disable wysiwyg on a rich text field ( I use wysiwyg module and tinymce 3.5.10 )
the media markup is lost. Also - existing embeds get lost on node save.
New embeds work in both disabling wysiwyg and saving nodes.

are there any missing steps? or do I have to save nodes between versions? or do I have to update to 7.x-2.0-alpha3 first?

When I load the node the image shows inside the editor, and when I click on the image the media button gets highlighted.
clicking on the media button returns this error:
Uncaught TypeError: Cannot read property 'fid' of undefined

Any ideas of where to go?

CommentFileSizeAuthor
#22 media-break_existing_content-2320535-22.patch589 bytesjustindodge
#7 media-duck_out_of_error-2320535-7.patch599 bytestom friedhof
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented
Issue summary: View changes
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented

Is it similar circumstances to #2317519: Blank WYSIWYG with existing multiple Media content?

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

The errors don't seem to be directly related, but a separate issue that also appears in my development site after the upgrade -
In most nodes with a few media embeds I don't get javascript errors before I try to edit the media embed.
I think this error probably is due to data corruption because of the issues that were addressed in the #2126755: Improve Media's WYSIWYG Macro handling issue.
Here is an excerpt of an example of body code with embeds:

<img src="https://www.nmbu.no/sites/default/files/wysiwyg_inserts/institutter.jpg" width="200" height="182" data-file_info="%7B%22fid%22:%223172%22,%22view_mode%22:%22media_original%22,%22fields%22:%7B%22format%22:%22media_original%22,%22field_file_image_alt_text%5Bund%5D%5B0%5D%5Bvalue%5D%22:%22%22,%22field_file_image_title_text%5Bund%5D%5B0%5D%5Bvalue%5D%22:%22%22,%22field_umb_image_caption%5Bnb%5D%5B0%5D%5Bvalue%5D%22:%22NMBU%20Institutter%20og%20forskergrupper%22,%22field_umb_image_tags%5Bund%5D%22:%22NMBU%22,%22field_umb_department%5Bund%5D%22:%22NMBU%22,%22field_umb_organization%5Bund%5D%22:%22Norges%20milj%C3%B8-%20og%20biovitenskapelige%20universitet%22,%22field_umb_image_credit%5Bund%5D%5B0%5D%5Bvalue%5D%22:%22NMBU%22,%22field_umb_image_geofield%5Bund%5D%5B0%5D%5Bgeom%5D%5Blat%5D%22:%22%22,%22field_umb_image_geofield%5Bund%5D%5B0%5D%5Bgeom%5D%5Blon%5D%22:%22%22%7D,%22type%22:%22media%22%7D">

The bug you refer to (#2317519: Blank WYSIWYG with existing multiple Media content is also appearing in my site, when I tested a post with multiple media embeds ( youtube video embeds)
I get this error when loading the node edit form, but the editor still works, but the next wysiwyg field on the form breaks:
"Uncaught TypeError: Cannot read property 'tagName' of undefined"

Might this tagmap related bug you mention be related to #2285865: Media module cannot replace placeholders in WYSIWYG if page contains more than one editor. which is also tagmap related.

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

@heddn : I do get the issue in #2317519: Blank WYSIWYG with existing multiple Media content consistently on nodes with more than one embed, and where the embed code is intact in the correct format:
[[{"fid":"4551","view_mode":"hvoedspaltebredde","fields":{"format":"hvoedspaltebredde","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","field_umb_image_caption[nb][0][value]":"Riving av gamle bygninger for å gi plass til ny barnehage i krysset Utvegen Kirkevegen.","field_umb_image_tags[und]":"Bygg","field_umb_department[und]":"Felles","field_umb_organization[und]":"Norges miljø- og biovitenskapelige universitet","field_umb_image_credit[und][0][value]":"Kai Einar Tilley","field_umb_image_geofield[und][0][geom][lat]":"","field_umb_image_geofield[und][0][geom][lon]":""},"type":"media","attributes":{}}]]l

Where the code has been scrambled, (or as I believe, is replaced by the wysiwyg preview img tag?) the error described in this issue.
Does it sound correct that the behaviour described in this issue is due to mangled embed code in the database?
Have anyone written a script / sql snippet that can revert embed code from image tags as described in #3 here?
other suggestions?

steinmb’s picture

steinmb CreditAttribution: steinmb commented
Title: upgrade from media 7.x-2.0-alpha2 to 7.x-2.x-dev » Media 7.x-2.0-alpha2 to 7.x-2.x-dev break existing WYSIWYG content
Priority: Critical » Major

Sounds more like a bug to me.

cbeier’s picture

cbeier CreditAttribution: cbeier commented

I can confirm this issue. I have updated media vom 7.x-2.0-alpha3+93-dev to 7.x-2.0-alpha3+108-dev. But in the older versions (+93-dev) the media markup somestimes brakes and the raw html was saved in the node.

The problem is general: I use CKeditor via WYSIWYG module.

With the new version, the elements are now removed, because no valid element in the tagmap was found.

Currently, I have changed in the media_wysiwyg.filter.js:

@@ -86,6 +86,10 @@
           // @see replaceTokenWidthPlaceholder()
           Drupal.settings.tagmap[macro] = Drupal.media.filter.outerHTML(el);
 
+          if (macro == false) {
+            return el[0].outerHTML;
+          }
+
           return macro;
         });
       }

The code is not perfect, because we replace (if no tag was found) the html code with the same html code.

In the new version (+108-dev) some issues are solved, but the old inserted ("invalid") media elements should not be removed.

But we should intercept the scenario reasonable, if not a macro was found. Removing content is always a bad idea. :)

tom friedhof’s picture

tom friedhof CreditAttribution: tom friedhof commented
FileSize
media-duck_out_of_error-2320535-7.patch599 bytes

I am getting this error message as well, with the latest dev version: "7.x-2.0-alpha3+108-dev".
"Uncaught TypeError: Cannot read property 'tagName' of undefined"

The error is completely breaking the page and leaving all the wysiwyg uninitialized. The attached patch does not fix the real problem, just allows each of the editors on the page to finish initializing.

Note: Your tokens will not be converted with this patch.

steinmb’s picture

steinmb CreditAttribution: steinmb commented
Category: Support request » Bug report
grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

@tom friedhof: I think you are describing this bug: #2317519: Blank WYSIWYG with existing multiple Media content.
In this bug, the wysiwygs initialize, but the media embeds (embedded pre update) are not there.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented
Status: Active » Postponed (maintainer needs more info)

Is this happening with a view mode that is an image, or is the view mode displaying a link instead?

notimetoexplaingetonthepony’s picture

notimetoexplaingetonthepony CreditAttribution: notimetoexplaingetonthepony commented

@heddn

I am willing to work with you on identifying and resolving the issue. The issue I am experiencing is similar to #3 where the already existing media content is in HTML markup and not a json macro which appears to be overwriting it with nothing.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented

re: #11
If you have existing markup, then there is little that can be done. But if it is still macro, then see #1792738: Allow custom file view modes for WYSIWYG display

Although if you are having issue opening the WYSIWYG, then maybe #7 will at least get you going.

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

@heddn, RE:#10 its images as mentioned.
RE: #12 its markup and not macros correct, markup that is now mostly disconnected from media module i think (unless it's preview markup? )
but the images disappears because they are removed. Before the update this non connected markup was left alone, still displaying images.
After the update they get removed.

This markup is still being created when images are dragged between positions in the content in the editor. this is fixed in #2126755: Improve Media's WYSIWYG Macro handling but I can't use the fix and stop the breaking of macros into markup because I then loose a over a thousand image embeds. For me I would finally be able to not get broken macros in nodes if the suggestion in #6 meant my existing markup images can be left in the node content. It sounds as a solution to me.

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

If its possible to salvage the embed macros from the data-file_info attribute somehow, a hint to how would enable me to contribute by asking our paid drupal consultants use our money to contribute here in such a direction. (they already contribute patches on s regular basis). Other suggestions for how they can contribute in this update blocker is also welcome.

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

Re: #10 its images inserted with a image view mode, not rendered as a link.

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

Do you need more info?

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented

Re #10:
I think that you have some type of HTML filter removing the img tags if they are getting stripped from the WYSIWYG. If the text field still has the img tags in the DB, that is probably your situation. If this is the case, then you have to update the text filter accordingly.

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

The only filters on my textformat are these:
Convert Media tags to markup
Correct URLs with Pathologic

However, when I try the suggested fix in comment #6, the old embeds work!
I can with this change in media_wysiwyg.filter.js save nodes with the above described preview html in place of macros.

Btw, the version I tried to update to in development server, that led to missing old embeds is 7.x-2.0-alpha3+100-dev

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

Do you need more info now?

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

Do you need more observations of where the embedded images are removed? Both @cbeier in comment #6 and me in comment #18 have tested the same code change to fix the issue. Should It be rolled into a patch?

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented

Sure, roll it into a patch and move status to needs review. Although #2328493: Async behavior of wysiwyg plugin attach/detach leads to some media tags not being replaced in time was recently committed and it might help things for you too.

justindodge’s picture

justindodge CreditAttribution: justindodge commented
FileSize
media-break_existing_content-2320535-22.patch589 bytes

Here's #6 in patch form, which may not be perfect but does resolve the glaring issue for me.
We've got WYSIWYG in combination with Ckeditor also, but it does seem to be a general issue.

I'm changing this to critical - all it takes for content editors on our site to completely destroy their content is to edit an existing node and resave, changing input formats or disabling rich text is not necessary. I'm not sure if that's been noted by others above - it may be related to having an even older version of media at one time, but this is really, really nasty.

justindodge’s picture

justindodge CreditAttribution: justindodge commented
Priority: Major » Critical
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented
Status: Postponed (maintainer needs more info) » Needs review
grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

I have tested the patch in #22 and it fixes the issue for me.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn commented

@grahamtk want to mark as reviewed and tested then?

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented
Status: Needs review » Reviewed & tested by the community

I would love to! Thanks, @heddn!

grahamtk’s picture

grahamtk CreditAttribution: grahamtk commented

Anyone to commit it?

  • aaron committed 247b31f on 7.x-2.x 
    Issue #2320535 by justindodge, tom friedhof: fixed Media 7.x-2.0-alpha2...
aaron’s picture

aaron CreditAttribution: aaron commented
Status: Reviewed & tested by the community » Fixed

Committed to http://drupalcode.org/project/media.git/commit/247b31f.

Status: Fixed » Closed (fixed)

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