Problem/Motivation

When embedding an image into the WYSIWYG via the add media button, if you alter the alt or title fields those changes are no longer rendered.

Tested with WYSIWYG module loading the CKEditor 4 library.

Proposed resolution

Lines 200-206 of media_wysiwyg.filter.inc refer to setting #alt and #title attribute overrides if they exist.

<?php
// Overwrite or set the file #alt attribute if it has been set in this
// instance.
if (!empty($element['content']['file']['#attributes']['alt'])) {
  $element['content']['file']['#alt'] = $element['content']['file']['#attributes']['alt'];
}
// Overwrite or set the file #title attribute if it has been set in this
// instance.
if (!empty($element['content']['file']['#attributes']['title'])) {
  $element['content']['file']['#title'] = $element['content']['file']['#attributes']['title'];
}
?>

However, I've found that to be the opposite. If these lines are removed, the alt and title overrides are rendered in the markup.

Remaining tasks

Discuss why if $wysiwyg is set to false in media_wysiwyg_token_to_markup we're setting $element['content']['file']['#alt'] and $element['content']['file']['#title'] to their 'default' values.
Provide patch that removes this code.
Provide screenshots fixing the regression.

User interface changes

None.

API changes

None.

Patch forthcoming.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bneil’s picture

Title: Alt and Title overrides do not override » WYSIWYG Alt and Title overrides do not override
bneil’s picture

Default values


<p>[[{"fid":"7704","view_mode":"full","fields":{"format":"full","field_file_image_alt_text[und][0][value]":"Kitten","field_file_image_title_text[und][0][value]":"A friendly cat"},"type":"media","attributes":{"alt":"Kitten","title":"A friendly cat","height":280,"width":480,"class":"media-element file-full"},"link_text":null}]]</p>

<img alt="Kitten" title="A friendly cat" height="280" width="480" class="media-element file-full image-style-large" typeof="foaf:Image" src="url/files/styles/large/public/kitten1.jpeg?itok=dT6oEgnA">

Overrides

Here I have overridden the alt and title fields.

Token and markup before patch

WYSIWYG token includes the overridden values.

<p>[[{"fid":"7704","view_mode":"full","fields":{"format":"full","field_file_image_alt_text[und][0][value]":"Kitten - override","field_file_image_title_text[und][0][value]":"A friendly cat - override"},"type":"media","attributes":{"alt":"Kitten","title":"A friendly cat","height":280,"width":480,"class":"media-element file-full"},"link_text":null}]]</p>

However, the rendered markup is still using the alt and title text saved on the file entity.

<img alt="Kitten" title="A friendly cat" height="280" width="480" class="media-element file-full image-style-large" typeof="foaf:Image" src="url/files/styles/large/public/kitten1.jpeg?itok=dT6oEgnA">

Token and markup after patch

The token continues to include the override values.
<p>[[{"fid":"7704","view_mode":"full","fields":{"format":"full","field_file_image_alt_text[und][0][value]":"Kitten - override","field_file_image_title_text[und][0][value]":"A friendly cat - override"},"type":"media","link_text":null,"attributes":{"alt":"Kitten","title":"A friendly cat","height":280,"width":480,"class":"media-element file-full"}}]]</p>
Now, the markup also includes the overridden values.
<img alt="Kitten - override" title="A friendly cat - override" height="280" width="480" class="media-element file-full image-style-large" typeof="foaf:Image" src=url/files/styles/large/public/kitten1.jpeg?itok=dT6oEgnA">

bneil’s picture

Assigned: bneil » Unassigned
Status: Active » Needs review
briand44’s picture

The patch applies cleanly and works as described. I'm not sure what role the ckeditor alt and title fields are suppose to play in this though.

mgifford’s picture

Issue tags: +Accessibility

What are the steps to reproduce this? Patch still applies nicely on SimplyTest.me.

durum’s picture

This also happens when you add an additional field to the file entity other than alt or title attributes. Do you think it is related?

aaronpinero’s picture

This patch did not work for me. The issue is probably that Media is not inserting tokens; in inserts HTML wrapped in a special tag. I'm not sure why this is the case. If anyone can help, I am using the latest dev version of the Media module and version 4.4.7 of CKEditor. I have tried numerous patches but cannot seem to get this to work. If I edit the entity that has the image and add alt and title information, that is perserved, but anything I try to put into the Media Browser dialog for addition into CKEditor is ignored.

Status: Needs review » Needs work

The last submitted patch, 2: media-wysiwyg-alt-title-overrides-2416701-2.patch, failed testing.

Dave Reid’s picture

I cannot reproduce this on the latest 7.x-2.x, even without the patch.

aaronpinero’s picture

I am using:

- Media Module 7.x-2.0-alpha4+37-dev
- CKEditor module (not WYSIWYG) 7.x-1.16
- CKEditor version 4.4.7

What follows are my internal notes and research on this problem, a description of specific files and functions in the chain of events for the embed of the media object, and the solution:

The specific modules used for this solution have a bug. When an image is being inserted into the CKEditor from the Media Library -- not from upload -- the fields provided to set alt and title attributes on the image were ignored, and the image was added to the content either without alt and title attributes or with the values from the file entity. This occurred because the names of the form element field names did not match the any of the legal HTML image tag attributes. The names instead came from the file entity object. The solution at this time is to patch the JavaScript for the module to add the alt and title text to the data for the inserted image using the correct attribute names.

media/modules/media_wysiwyg/includes/media_wysiwyg.pages.inc
file where form elements from modal are created from fields in the entity

media/modules/media_wysiwyg/wysiwyg_plugins/media_ckeditor/library.js
insertMediaFile function takes information from the entity and the media modal window and passes it to Drupal.media.filter.create_element()
it is here where I have chosen to make the modification to the information from the modal (formattedMedia.options object) to properly name the alt and title properties

patch at line 45

 if (formattedMedia.options["field_file_image_alt_text[und][0][value]"] != undefined) {
 	if (formattedMedia.options["field_file_image_alt_text[und][0][value]"] != "") {
 		formattedMedia.options.alt = formattedMedia.options["field_file_image_alt_text[und][0][value]"];
 	}
 }
 if (formattedMedia.options["field_file_image_title_text[und][0][value]"] != undefined) {
 	if (formattedMedia.options["field_file_image_title_text[und][0][value]"] != "") {
 		formattedMedia.options.title = formattedMedia.options["field_file_image_title_text[und][0][value]"];
 	}
 }

media/modules/media_wysiwyg/js/media_wysiwyg.filter.js
create_element function reads the attributes from formattedMedia.options and turns them into attributes for the image element
this process is based on Drupal.settings.media.wysiwyg_allowed_attributes

media/module/media_wysiwyg/media_wysiwyg.module
function _media_wysiwyg_wysiwyg_allowed_attributes_default() has the list of allowed attributes

rooby’s picture

I'm using:
CKEditor plugin: 4.5.3
ckeditor (standalone module): 7.x-1.16+9-dev
media: 7.x-2.0-beta1
media_ckeditor: 7.x-2.0-alpha1
file_entity: 7.x-2.0-beta2

I'm currently not using any patches.

It seems to be all working okay except for this issue.

The issue for me is that if I select a previously uploaded image from the media library and override the alt or title text it doesn't work. Instead I get whatever text was originally entered for that image.

Does this issue belong here or in the media_ckeditor module?

bneil’s picture

Status: Needs work » Needs review
FileSize
1.34 KB

Here's a version of the patch that should apply cleanly to head.

bneil’s picture

@Dave Reid - I'm still able to replicate it via simplytest.me and the media_dev profile.

@rooby - since there are people using ckeditor module and others using wysiwyg module, I think we should keep it here in media wysiwyg. I think moving it to media_ckeditor would imply that the issue is a ckeditor module-related issue and it's larger than that.

siramsay’s picture

**********
Below is now not relevant as I found a solution #20. even though this small change did half fix the problem as noted
**********
I applied the patch with no luck, exact same problem so won't repeat.

I am using
File Entity (fieldable files) 7.x-2.0-beta2+13-dev (2015-Oct-04)
Media 7.x-2.0-beta1+9-dev (2015-Oct-02)
- Media CKEditor 7.x-2.0-alpha1+4-dev (2015-Aug-27)
CKEditor - WYSIWYG HTML editor 7.x-1.16+14-dev (2015-Oct-09)
CKEditor 4.5.3

#11 says to patch media/modules/media_wysiwyg/wysiwyg_plugins/media_ckeditor/library.js
this has now moved to media_ckeditor/js/plugins/media/library.js

I have gone a head and patched media_ckeditor/js/plugins/media/library.js as suggested in #11 and it works , overides are retained, below is the code, starting at line 43.

However when you click on the add media button the overrides are lost so still needs work.

insertMediaFile: function (mediaFile, formattedMedia, ckeditorInstance) {
      // Customization of Drupal.media.filter.registerNewElement().
      // begin patch line 45
	if (formattedMedia.options["field_file_image_alt_text[und][0][value]"] != undefined) {
 		if (formattedMedia.options["field_file_image_alt_text[und][0][value]"] != "") {
 			formattedMedia.options.alt = formattedMedia.options["field_file_image_alt_text[und][0][value]"];
 			}
 		}
	if (formattedMedia.options["field_file_image_title_text[und][0][value]"] != undefined) {
 		if (formattedMedia.options["field_file_image_title_text[und][0][value]"] != "") {
 			formattedMedia.options.title = formattedMedia.options["field_file_image_title_text[und][0][value]"];
 			}
 		}
	 // end patch
	  var element = Drupal.media.filter.create_element(formattedMedia.html, {

-----
other relevant references
I added the reference issue which is related to a WYSIWYG ckeditor issue. which has a solution somewhat.

This is also related and was committed but then removed?
Add ability to edit file meta data and fields when embedded in WYSIWYG https://www.drupal.org/node/835516

And this is also related and mentions the above
Edit media instance in Wysiwyg field https://www.drupal.org/node/1404084

vistree’s picture

Hi, tried patch from #13 with no luck. Tested with
wysiwyg 7.x-2.2+70-dev
ckeditor 4.4.7.3a35b3d
media 7.x-2.0-beta1+9-dev
file entity 7.x-2.0-beta2

I enabled Media WYSIWYG View Mode (don't know it this can have side effects) and created custom view modes by custom module.

I can add / or edit an embeded media (image) but the changes are not stored in the media tag!

siramsay’s picture

@vistree if you are using wysiwyg module I think you need to go to this forum https://www.drupal.org/node/2126697 Wysiwyg -- Alt and Title fields require some special handling.

this thread is related to the set up I have in #15

vistree’s picture

Ah, thanx for clarification and the link!!!

siramsay’s picture

Status: Needs review » Needs work
siramsay’s picture

As well as adding the the code in #15 to media_ckeditor/js/plugins/media/library.js

** 21 Jun 2016 Edit 1 - patch at #30 works so best to use that instead of patch #83 noted below on drupal/node/2062721 - still need patch https://www.drupal.org/node/2352573 **

** 21 Jun 2016 Edit 2 - With this solution I found a bug that if you double clicked on an image the image properties dialog opens up and then overrides don't work (as per the original issue?). Solution was to remove image plugin in advanced options of CK editor profile settings so image properties dialog doesn't open- config.removePlugins = ‘image'; **

for me I found that the below patches were needed to solve this issue, nothing from this issue thread was needed

you need to patched media_ckeditor/js/plugins/media/library.js as per this issue
CKEditor Media WYSIWYG library.js does not pass entity fields into create_element (2352573)
https://www.drupal.org/files/issues/ckeditor_media_wysiwyg-2352573-7.patch

Also patching media is needed as in this issue
Add a white list of file fields that can be overwritten when the file is added in the wysiwyg (2062721)
#83 https://www.drupal.org/node/2062721#comment-10296481 https://www.drupal.org/files/issues/media-wysiwyg-override-white-list-20...

after patching
1- run update
2- overide alts & title again and save node

tested on 3 installs and seems fixed as per module set up I listed below (most current of all dev)

File Entity (fieldable files) 7.x-2.beta2 +13dev (2015-Oct-04)
Media 7.x-2.0-beta1 + 9-dev (2015-Oct-02)
- Media CKEditor 7.x-2.0-alpha1+4-dev (2015-Aug-27)
CKEditor - WYSIWYG HTML editor 7.x-1.16+15-dev (2015-Dec-01)
ckeditor 4.5.3

siramsay’s picture

Status: Needs work » Needs review

maybe needs to be marked duplicate?

rooby’s picture

The patch in #13 didn't work for me.

I also removed that and tried the setup outlined in #20 and that didn't work for me either.

The file alt/title text didn't work and if I overrode those when inserting into wysiwyg they didn't work either.

I always end up with alt="" title="".

With the setup from #20 I can see the title and alt text in the media token though.

My versions are:

  • File Entity 7.x-2.0-beta2+13-dev (2015-10-03)
  • Media 7.x-2.0-beta1+9-dev (2015-10-01)
  • Media CKEditor 7.x-2.0-alpha1+4-dev (2015-08-26)
  • CKEditor 7.x-1.16+15-dev (2015-11-30)
  • CKEditor library 4.5.6 (without the widget plugin)
rooby’s picture

I have seemingly got the solution in #20 working now, with this new version of the whitelist patch: #2062721-89: Add a white list of file fields that can be overwritten when the file is added in the wysiwyg

aaronpinero’s picture

I would like the maintainers to address an important consideration related to this issue: the full UX of adding and editing images in the WYSIWYG

- selecting an item from the media library; in my case, an image
- being able to customize the alt and title text of that image (including having them be blank) before insertion into the WYSIWYG
- editing that image by selecting the image and opening the media library dialog and having the customizations of format, alt text, and title text be preserved

Right now, the last two do not work. And I have not seen, in any of the threads related to this issue, where the last part is being addressed.

Adding images, with proper alt and title text, into the WYSIWYG, is a very important use case for every Drupal website I've ever worked on. I cannot imagine a client accepting a website where this was not only possible but worked without flaw. And yet, in the couple of years of life of Media module, I have never had this work properly. Some sort of workaround or hack or patch or addition of other modules (like Insert) is necessary. It would be nice to see this problem receive some serious consideration, rather than a lengthy list of patches that may or may not resolve the issue. I realize this is a complex module with lots of pieces, but the UX for the content editors seems (given all the issues related to this problem) seems to get the short straw.

Meanwhile, I'm still working on my own recipe for this. I've solved the problem with the second step (it's actually a really simple edit to the js for Media WYSIWYG which I will post later), and should be able to fix the third step. I'll post my recommendations and the full module recipe once I'm finished.

aaronpinero’s picture

Okay, here is my recipe for making this work:

Modules:
- File Entity 7.x-2.0-beta2
- Media 7.x-2.0-beta1+10-dev
- Media CKEditor 7.x-2.0-alpha1
- CKEditor 7.x-1.17
- Entity view modes 7.x-1.0-rc1 (I create custom view modes for images being inserted into CKEditor)

Libraries:
- CKEditor 4.5.7

Analysis of the problem:

When using Media with CKEditor, a widget is created that allows users to insert entities from the Media library into the CKEditor WYSIWYG. For images in particular, after you upload an image or select one from the library, you can select a view mode, modify the alt text and modify the title text.

the image options in the Media dialog

There are two problems related to the interaction with this dialog.

(1) The name attributes on the input elements for alt text and title text do not match legal HTML IMG tag attributes. They names are actually "field_file_image_alt_text[und][0][value]" and "field_file_image_title_text[und][0][value]". This makes sense because they are field values for the media entity in question. However, Media WYSIWYG doesn't provide necessary translation of these values to "alt" and "title" attributes for a IMG tag. Instead, the module tries to create attributes for the IMG tag with the names "field_file_image_alt_text[und][0][value]" and "field_file_image_title_text[und][0][value]". These are not legal names, so Media WYSIWYG doesn't recognize the values of these inputs and instead reverts the values to whatever are the default values for the selected media entity.

(2) Users should have the ability to edit an image that was placed into CKEditor using the Media widget. A user can select an image in the WYSIWYG and click on the Media widget. The Media dialog opens to show the options for the selected image. However, it does not show/preserve the existing values for format, alt text, or title text. Instead, it reverts to the default format, alt text, and title text for the selected media entity. This occurs because the Drupal.media.popups.mediaStyleSelector function expects a mediaFile object with a fields parameter. Instead, it is being passed an object where the data is in an attributes parameter. This looks like a simple case of two developers not agreeing on a naming convention.

I've been able to fix both problems with relatively small, simple patches to three js files in the Media Module. I am describing the pages below and have also attached patch files.

Patch to media/modules/media_wysiwyg/js/media_wysiwyg.format_form.js: starting line 39, insert

/// patch start    
    if (field.name.indexOf("alt_text") != -1) {
	    ret['alt'] = field.value;
    }
    if (field.name.indexOf("title_text") != -1) {
	    ret['title'] = field.value;
    }
/// patch end

This patch assigns the alt text and title text to properly named parameters in the ret object before being returned by the Drupal.media.formatForm.getOptions function.

Patch to media/modules/media_wysiwyg/js/media_wysiwyg.filter.js: starting line 220, insert

/// patch start
          file_info.attributes['field_file_image_alt_text[und][0][value]'] = file_info.attributes['alt'];
          file_info.attributes['field_file_image_title_text[und][0][value]'] = file_info.attributes['title'];
/// patch end

This patch completes the round trip of data started by the previous patch. If the user selects an image in the WYSIWYG to edit, we want the alt and title text for the selected image to be reflected in the form fields. This requires assigning attributes to the file_info object that have names matching the form input names.

Patch to media/js/media.popups.js: starting line 186, insert

/// patch start    
  if (mediaFile.attributes !== undefined) {
	  mediaFile.fields = mediaFile.attributes;
	}
  if (mediaFile.fields !== undefined) {
	  mediaFile.fields.format = mediaFile.view_mode;
	}
/// patch end

This patch makes sure that the data about the existing media element that the Drupal.media.popups.mediaStyleSelector function is looking for is present in the correct part of the mediaFile object.

The last submitted patch, 26: media_wysiwyg-filter-js.patch, failed testing.

The last submitted patch, 26: media_wysiwyg-format_form-js.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 26: media-popups-js.patch, failed testing.

brockfanning’s picture

#13 is not fixing the issue for me, and #26 has hardcoded references to "field_file_image_alt_text" and such, which should be considered a variable thing (since the file_entity allows customization of the alt/title using tokens). At this point in the Drupal 7 Media lifecycle, I'm most interested in the shortest patch that gets this thing working. So here is my attempt, attached.

siramsay’s picture

Patch #30 works nicely.

File Entity (fieldable files) 7.x-2.beta3 +2dev (2016-Jun-02
Media 7.x-2.0-beta2+1-dev (2016-Jun-02)
- Media CKEditor 7.x-2.0-alpha1+4-dev (2015-Aug-27)
CKEditor - WYSIWYG HTML editor 7.x-1.17

note* I find it strange that double click on image opens Image Properties dialog and not Media dialog and these are not synced so have disabled this in CK Editor profiles advanced (config.removePlugins = ‘image'; ) as when creating an new piece of content this is sure to happen with a content creator.

aaronpinero’s picture

In other threads, I learned that, basically, the CKEditor module won't work with the Media module (version 7.x-2.x). There are just too many problems. So, I switched to using the latest development version of the WYSIWYG module with is compatible with CKEditor version 4 (the latest recommended release is not). So, for the record, I am now using:

media 7.x-2.0-beta2
enable mediafield, media_wysiwyg, media_wysiwyg_view_mode
file_entity 7.x-2.0-beta3
wysiwyg 7.x-2.2+76-dev
ckeditor 4.4.8 (full package)

The interaction between Media and CKEditor works better when using the WYSIWYG module as a mediator, but there is still the same problem with the alt and title tags when inserting media using CKEditor.

I was able to isolate the problem to one of the files in the media_wysiwyg module and prepare the attached patch. This thankfully much cleaner than my last patch attempt. This set of modules basically works as one might expect after the input format filters have been properly configured.

I am working on a Github project (https://github.com/aaronpinero/drupal_wysiwyg) that documents and provides a sample of the working configuration.

brockfanning’s picture

@aaronpinero: An issue I see is that the Media module provides a config form where admins can control the token that is used for alt and title. So it wouldn't be inconceivable for a site to be using completely different fields to control alt and title. For this reason I steered clear of hard-coding a particular field name, or even part of a field name.

Side note: I've been putting in a lot of work lately to try to get Media and Ckeditor working together. My current solution does in fact use the Ckeditor module, and the latest Ckeditor library, and I've been trying to document it here: #2730285: A working Media + Ckeditor recipe

siramsay’s picture

just an update on modules used with #30

File Entity (fieldable files) 7.x-2.0-beta3+3-dev (2016-Jun-22)
Media 7.x-2.0-beta2
- Media CKEditor 7.x-2.0-alpha1+4-dev (2015-Aug-27)
CKEditor - WYSIWYG HTML editor 7.x-1.17

siramsay’s picture

I have tested this and no longer need this patch as per set up listed below, tested on about 8 installs all with most current core 7.51

File Entity (fieldable files) 7.x-2.0-beta3+3-dev (2016-Jun-22)
Media 7.x-2.0-beta5
- Media CKEditor 7.x-2.0-alpha1+4-dev (2015-Aug-27)
CKEditor - WYSIWYG HTML editor 7.x-1.17

I still have this patch for ckeditor in place but at present unsure if still needed CKEditor Plugin - Field values CKEditor Plugin - Field values https://www.drupal.org/node/2455557

this is related to my comment in #20 that states
"you need to patched media_ckeditor/js/plugins/media/library.js as per this issue
CKEditor Media WYSIWYG library.js does not pass entity fields into create_element (2352573) which is now close as a duplicate of the above

joseph.olstad’s picture

siramsay’s picture

sorry to confuse you @joseph.olstad

#2352573: CKEditor Media WYSIWYG library.js does not pass entity fields into create_element is closed as a duplicate of #2455557: CKEditor Plugin - Field values which is RTBC

I just tested removal of patch at #2455557: CKEditor Plugin - Field values https://www.drupal.org/files/issues/2455557-4.patch and it is still needed as field over rides are still not passed when opening an over ridden image using the add Media button in the WYSIWYG.

joseph.olstad’s picture

ok so unless the maintainers of media_ckeditor take action we will need someone to take ownership of the media_ckeditor project asap and get that patch 4 from #2455557: CKEditor Plugin - Field values put into a release.

@size , and everyone else, please contact all the maintainers of media_ckeditor and ask them kindly to do this, if no timely response followup here.

Meanwhile probably a good idea to open a ticket offering to maintain the media_ckeditor module.

joseph.olstad’s picture

Status: Needs work » Fixed

download the latest release of media_ckeditor

Status: Fixed » Closed (fixed)

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