Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.From comments https://drupal.org/comment/8281403#comment-8281403 and https://drupal.org/comment/8346709#comment-8346709, it appears that using a WYSIWYG field on an image field (title, for example) causes the data in that field to be un-updatable.
I've got a patch ready to be rolled and will be posting it shortly.
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | media-wysiwyg-fields-with-wysiwyg-2170759-23.patch | 791 bytes | ksenzee |
| #20 | media-wysiwyg-fields-with-wysiwyg-2170759-20.patch | 541 bytes | benjaminbradley |
| #14 | media-wysiwyg-fields-with-wysiwyg-2170759-14.patch | 3.44 KB | GoZ |
| #11 | media-wysiwyg-rich-fields-2170759-11.patch | 3.32 KB | kaidjohnson |
Comments
Comment #1
kaidjohnson CreditAttribution: kaidjohnson commentedProposed patch...
The only issue I am running into at this point is if I take the original field_file_image_title_text (which is set to a 'text' field, not a 'long text' field) and enable the wysiwyg for it. The values DO pass into the macro, however the wysiwyg editor for this text field is not populated with the value properly. I believe this is an issue with using Wysiwyg on a text field, but I can't be entirely sure. Tested with a new 'Caption' field which is a 'long text' field and it worked 100% as expected.
Comment #2
kaidjohnson CreditAttribution: kaidjohnson commentedStatus...
Comment #4
alexkb CreditAttribution: alexkb commentedHi Kai, thanks for posting this patch!
Are you noticing that on the first load of the rich text editor, the overridden image settings (stored in the macro correctly) aren't loaded into the media popup the first time? Once you set it again and edit it again it loads and edits fine though, so thank you for this fix.
Additionally we have a weird side effect where if you click the ckeditor source code button after doing a media edit, and then go back to non-source code mode and click media edit again, media resets all its fields.
This aside, the only other issue now is that the markup in the rich text area is being encoded. This started happening in your patch in the other thread. As we have a lot of content stored prior to this patch so we can't apply it (with an decode on the render side) yet. I'm thinking of disabling the old input format, and then creating another input text format and then moving forward we only decode image captions in this new format.
Comment #5
alexkb CreditAttribution: alexkb commentedOk, those first two issues with media's fields being reset is because the markup in the caption has been entity encoded but not entirely safely encoded for a URL argument (which in turn gets passed to media_wysiwyg_format_form() and then bungs out when it can't process $_GET['fields'] properly, so all fields are set to default.
Comment #6
kaidjohnson CreditAttribution: kaidjohnson commented@alexkb - see https://drupal.org/comment/8364441#comment-8364441 regarding the encoding. Let me know if the patch in #45 (on that thread) resolves that piece of the issue for you. It's possible that all of the issue you outline here in #4 and #5 will be resolved in the other thread, although I'm not entirely sure I follow you for some of it.
Comment #7
alexkb CreditAttribution: alexkb commentedhttps://drupal.org/comment/8364803#comment-8364803
Comment #8
kaidjohnson CreditAttribution: kaidjohnson commented1: media-wysiwyg-fields-with-wysiwyg-2170759-1.patch queued for re-testing.
Comment #9
alexkb CreditAttribution: alexkb commentedIf the media fields contain an ampersand e.g. if the user puts a nbsp or copyright character in their caption markup, then the settings stored in the macro don't get received by media_wysiwyg_format_form(). So perhaps, we do need to encode them after all?
Comment #10
alexkb CreditAttribution: alexkb commentedOk, I've rerolled your patch with an addition to wysiwyg-media.js which encodes the url parameters before it passes them to the media popout. Seems to work well in my initial tests. I'll get it tested more in another 24 hours with a colleague. Cheers.
Comment #11
kaidjohnson CreditAttribution: kaidjohnson commented@alexkb - it occurred to me that if a user wants to have classes or any other attributes in their markup within a caption field, those pesky double-quotes aren't happy being nested within markup tags. The WYISWYG wants to make sure the markup is correct so the JSON escaped quotes get re-wrapped in real quotes and bad things start to happen. Any rich-text field will need to be fully escaped in the JSON, which means a bit of encoding and decoding. I've found a rather unpleasant way to work around these two issues. You should be able to have complex markup in your caption field with this update.
Here was the test issue that was failing and caused this bit to be updated:
1) Open Media popup for any image with field(s) that use rich-text (wysiwyg) markup.
2) Put something simple into the rich-text field, such as
foo.3) 'Disable rich-text' on the image field, you should see
<p>foo</p>4) Add a class of 'bar' to the p tag:
<p class="bar">foo</p>5) 'Submit' the field form. At this point everything should appear to be fine.
6) 'Disable rich-text' on the field that contains the image you just updated. You should see the media macro with the
<p class="bar">foo</p>in its caption field.7) Here's where it fails: 'Enable rich-text'. :/ The double-quotes within the markup cause the macro to escape too early and now the media macro is broken and the image can't be re-rendered.
There were a handful of subtle issues in resolving it, namely that if the double-quotes weren't escaped before encoding the value in the macro, the editor and/or the media popup would try to put the double-quotes back in once it saw the class="bar" and more odd things would happen. The only thing I wasn't able to do was to display the value of the rich-text field(s) in a usable way from the macro side of things, which should be ok. Users can technically edit that area, but it makes much more sense to update their fields via the field editor in the media popup.
I believe that your fields, as they exist now (not encoded), will be ok and will be able to smoothly be updated without issue during a content update process; in other words, this update should be backwards compatible, but correct me if I'm wrong on that one.
Note also that this patch drastically cleans up the media_wysiwyg_filter_field_parser() function to not be dependent on field data that is a certain depth and urldecodes() the field values to allow the rest of this update to render properly.
Comment #12
alexkb CreditAttribution: alexkb commented@kaidjohnson, we were having some issues in IE8 when the wysiwyg seemed to automatically close/open nested <p> tags, causing our macros to stop being rendered. We had nested <p> tags in the first place, because it was a paragraph with the macro in it, and the caption with its own <p> tags.
Anyway, after applying your #11 patch, the problem went away, since you're encoding things now.
I was initially concerned with this patch, because it was altering media_wysiwyg_filter_field_parser() again, which might mess up existing unencoded macros. For whatever reason though, this doesn't happen, and the old and new macros work fine.
Comment #13
aasarava CreditAttribution: aasarava commentedThanks, the patch in #11 works to fix the problem I was having with the text inside a rich-text (ckeditor) caption field always disappearing when inserting media into a ckeditor body field. (Using the latest dev of the media_wysiwyg module.)
Comment #14
GoZ CreditAttribution: GoZ commentedit works now with latest 7.x-2.x branch commit 9583d89e810312ee76abb5ba9ac9174cecb7d815
Comment #15
samhassell CreditAttribution: samhassell commentedI've tested this patch on a vanilla install. WYSIWYG file entity fields are now updating and displaying correctly. Might be an idea to wait until #2126755: Improve Media's WYSIWYG Macro handling gets committed though.
Comment #18
aaron CreditAttribution: aaron commentedCommitted to http://drupalcode.org/project/media.git/commit/507f4c3.
Comment #20
benjaminbradley CreditAttribution: benjaminbradley commentedHi all,
I'm running into a problem introduced by this patch, not sure of the best way to handle (new issue or reopen this one?)
Symptom:
Image file type has a Filtered Text caption field
When Image token with caption is inserted from pop-up widget into page's wysiwyg editor, the caption's value is mistakenly quote-escaped prior to being urlencoded. Result: Caption HTML from widget (source =
<p>Caption <a href="http://example.com">link</a> example.</p>)changes to (source =
<p>Caption <a href=\"http://example.com\">link</a> example.</p>) -- notice extra backslashes added before doublequotes.When the pop-up widget is opened again, the anchor tag's URL is not parsed correctly and the URL destination is lost.
I think the problem is with the last couple lines in the patch from #14:
I'm attaching a patch here to remove that extra quote-escaping (apply patch #14 and then this one to latest release; or apply this patch only to latest dev).
Comment #21
Ivan Zugec CreditAttribution: Ivan Zugec commentedI'm also experiencing issue mentioned in #20. I have a caption field on the image file entity which allows text to be entered via filtered_html text format. The escaped double-quotes is breaking all links.
Example:
Comment #23
ksenzeeI have the same problem, and this patch fixes it. The test case from #11 passes, too. I'm guessing that the quote escaping was needed before #2126755: Improve Media's WYSIWYG Macro handling went in, but now the quotes are being needlessly double-escaped.
Rerolling the patch and updating the comment to reflect that we're no longer escaping quotes separately.
Comment #24
Devin Carlson CreditAttribution: Devin Carlson commentedTested #23 and committed to Media 7.x-2.x. Thanks!