Problem/Motivation

For files embedded in the WYSIWYG It should be possible to edit file meta data and fields on a per instance basis, e.g. Alt/title text, a caption field, etc.

Proposed resolution

In the WYSIWYG, when selecting a file and clicking the media button, a user should be provided with a form to edit these fields. We should also make it clear to the user that the changes are not global in scope, but only for this specific instance of the file.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rootwork’s picture

Subscribe (and +1)

Dave Reid’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Dave Reid’s picture

Component: User interface » WYSIWYG integration
Assigned: Unassigned » Dave Reid
Dave Reid’s picture

FileSize
71.61 KB

Ideally I'd like this to work similar to how Wordpress' WYSIWYG handles editing an image (see screenshot). We provide a contextual hovered image over the embedded image in the WYSIYWG, probably using our 'contextual' gear icon in core with an 'Edit dropdown' when clicked will open a CTools modal of file/[file:fid]/edit/js to edit the fields on a file. If a dropdown is too hard to do maybe just clicking the gear will edit the image's fields.

Selection_012.png

Dave Reid’s picture

Title: Make right-click menu for media to allow editing (and replacing) existing items » Add ability to edit file meta data and fields when embedded in WYISWYG
Dave Reid’s picture

Assigned: Dave Reid » becw
Devin Carlson’s picture

girishmuraly’s picture

There is a bit of implementation detail mentioned in #1404084: Edit media instance in Wysiwyg field for this.

joelcollinsdc’s picture

Not much forward progress on this or related issues recently. This is a pretty major usability concern, right?

Are there workarounds people are using in the meantime to accomplish metadata editing? Like, for example, inserting media using the media button but actually editing the metadata using their editor's old-fashioned image insert button?

ShaunDychko’s picture

FileSize
42.54 KB
15.23 KB

By clicking an inserted image, then clicking the media button, you can edit the "alt" and "title" fields, but any other fields are missing. The extra caption text field is available on the original "upload" screen, just not when the image is edited. Screenshots attached.

ShaunDychko’s picture

Title: Add ability to edit file meta data and fields when embedded in WYISWYG » Add ability to edit file meta data and fields when embedded in WYSIWYG
gmclelland’s picture

Another thing you might want to do is highlight the "media button" in the wysiwyg toolbar when a media item is selected. When clicking that highlighted media button it could popup the edit form for that file.

greg.1.anderson’s picture

I also need a simple way to edit image metadata when the image is embedded. Everything works great when the media image file is a field; I need the same metadata-editing capabilities for embedded images, though.

The media file "edit" button is handled by the function media_file_edit_modal, which essentially just calls through to file_entity_edit. Inspecting the implementation of this later function, I threw together this simple hook:

function MYMODULE_media_format_form_prepare_alter(&$form, $form_state, $file) {
  if ($file->type === 'image') {
    field_attach_form('file', $file, $form, $form_state, LANGUAGE_NONE);
    unset($form['field_file_image_alt_text']);
    unset($form['field_file_image_title_text']);
  }
}

The 'unset's are there because field_attach_form will add all fields of the form, including 'alt' and 'title', which already has these fields inside it. The above hook works great vis-a-vis the display of the fields, but of course it does not save the values once the user hits 'submit'. A little more code should take care of this; I do not have time to continue tonight, but will continue with this later.

Perhaps a more generalized implementation of this same technique might be used in the media module to include additional metadata fields for all embedded media file types. Something needs to be done about LANGUAGE_NONE, and a more general solution for duplicate fields in the dialog would also be necessary, but it seems this might be an easier solution than adding new buttons in the WYSIWYG editor, or floating on top of the media itself.

greg.1.anderson’s picture

Handling the form submit was more complicated than I anticipated. c.f. http://drupal.stackexchange.com/questions/62727/attach-a-submit-handler-...

girishmuraly’s picture

I am using the method described in #1404084: Edit media instance in Wysiwyg field and is usable enough for 5 of our sites.

rootwork’s picture

@girishmuraly does that still work with current versions of Media?

greg.1.anderson’s picture

@girishmuraly: looks useful, but my wysiwyg + js foo is weak, so I didn't follow all of the instructions in the post you referenced. Could you post a patch that shows everything all hooked up as you describe?

girishmuraly’s picture

FileSize
4.9 KB

@rootwork If I remember correctly, it worked well when I tested with 7.x-2.0-unstable7. However the code we are using in production is pinned to this commit from 05 June 2012.

@greg Am attaching a patch here that works on our sites which are built on a D7 distribution, with the version of media module mentioned above. Haven't tested it outside our distro and my js is pretty weak too, but hope it helps.

girishmuraly’s picture

Attached screenshots of how it works for us with the patch in #18. You need the media button enabled in your wysiwyg settings to embed the image in the body.

gmclelland’s picture

@girishmuraly - is this for TinyMCE only? or will it work with CKeditor as well?

girishmuraly’s picture

I haven't tried on CKeditor.

greg.1.anderson’s picture

FileSize
4.94 KB

Here is #19, re-rolled for today's 7.x-2.x dev branch.

media.image.overlay.js makes a direct call to tinymce.create, which means that it will fail with CKeditor (you get no editor, just a blank region on the screen where your editor should be). I need to use this with CKeditor, because I am currently using the ckeditor_link module.

I've only looked into updating it a little. It looks like CKEDITOR.plugins.add is the equivalent function for timymce.create, and CKEDITOR.dom can probably (?) be used in place of tinymce.DOM. CKEDITOR.currentInstance looks like a suitable equivalent to tinyMCE.activeEditor. There are probably a bunch of other changes that will need to be made.

Here is how media_crop adds its CKEDITOR plugin (only CKeditor supported):

/**
 * Implements hook_wysiwyg_plugin().
 */
function media_crop_wysiwyg_plugin($editor, $version) {
  switch ($editor) {
    case 'ckeditor':
      if ($version > 3) {
        return array(
          'media_crop_ckeditor' => array(
            'path' => drupal_get_path('module', 'media_crop') . '/plugins/ckeditor',
            'extensions' => array(
              'media_crop_edit_instance' => t('Media crop edit instance', array(), array('context' => 'Media crop')),
            ),
            'load' => TRUE,
          ),
        );
      }
      break;
  }
}

Could this same technique be used in place of drupal_add_js(drupal_get_path('module', 'media') . '/js/plugins/media.image.overlay.js'); to select the appropriate plugin for the active editor? I will fiddle with this as time permits, but I have no prior experience with extending wysiwyg editors.

greg.1.anderson’s picture

I installed tinymce 3.5.8 and tried to test #22. I found that with the patch installed, it was not possible to configure the Tinymce editor in the Wysiwyg module. Removing the patch, I could configure Tinymce, but re-applying the patch did not cause the image edit icon to be added to any embedded images. Either something has changed in the Media module, or I did not re-roll the patch correctly.

I have not yet tested with the commit specified in #18.

sylus’s picture

I managed to get it haphazardly working by changing the regex and leveraging data-file_info vs the class:

Edited media.image.overlay.js

26  var classesString = ed.dom.getAttrib(ed.selection.getNode(), 'data-file_info');
27  // Get the file id (fid) of the image from the class name (format 'img__fid__123')
28  var matchesArray = classesString.match(/fid%22:%22(\d+)%22/);

Also had to switch out media-image to media-element:

49  if ( ed.dom.getAttrib(e.target, 'class').indexOf('media-element') !== -1 ) {
greg.1.anderson’s picture

Testing #18, I get:

Fatal error: Call to undefined function file_info_file_types() in media.module on line 1053

I am guessing that I need an older version of some other module that used to define file_info_file_types for this to work.

girishmuraly’s picture

Yes file_info_file_types() used to be in the media module. I guess it got removed in the latest version.

Thanks for updating the patch.

greg.1.anderson’s picture

Odd; I thought I checked out the commit specified in #18 before applying the patch.

girishmuraly’s picture

It might help if I state I am having file_entity module on 2.0-unstable6.

Earlier, the function was defined there - had http://drupalcode.org/project/file_entity.git/blob/ba18f8b03c6c95692a678...

But it got removed by Nov 2012 - http://drupalcode.org/project/file_entity.git/blob/86580fef8c3706e3fe378...

And confusing is the fact that the same function was part of media module - http://drupalcode.org/project/media.git/blob/d564d33c0dd725c5758ed7620cf... but I'm unable to workout the trace of the function movements.

greg.1.anderson’s picture

I had to give up here because I don't know jQuery enough to solve the issues here in the time I have. I may return later, but for now I am happily using image fields instead of embedded images.

I wanted to add one more note on an important UX issue here, though. The existing media browser code displays the field data that is attached to the image file field; however, it saves edits into the media filter text that is embedded in the WYSIWYG text. It is the later that is actually rendered. Clearly, the browser should also extract the information from the filter text, and display that information in the metadata editing dialog. The important UX issue relates to the scope of the data: the media filter text is saved with the reference to the media file, whereas the media file fields themselves are, of course, attached to the actual media file (field). The upshot of this is that if you edit a field of a media file field, then the change will happen globally for all places where that image appears on your site. If, on the other hand, you edit a field of an embedded media file, the change will apply only to that one usage of the image.

For fields such as "caption", it makes sense to be able to have separate captions for each usage of an image. For my particular use case, though, I need edits to certain fields on the image to apply globally to all places where the image may be used. There are two problems with this requirement:

1) Global updates for embedded media file images is hard. It would be necessary to either seek out and alter all of the fields that contained embedded references to the media file image at save time, or alternatively, it would be necessary to fix up the embedded references at load or render time.

2) If the issues in (1) were solved, it would be necessary to allow the user in some way to be able to specify whether the attached media image fields should be saved at a global or local scope for embedded images.

I am thinking that since (1) is hard, it may become a necessary limitation of embedded images that their metadata is always local in scope. This would relegate folks who need globally scoped metadata to use media image fields only, and forgo embedded images. Perhaps there is a better solution, though.

aaron’s picture

Status: Active » Needs review
aaron’s picture

I have taken a different approach here, and I am attempting to add the field data to the form where you select the file display mode, where we already have the image alt and title text. I had limited success, with the following code. However, I have not yet figured out how to store the data on submission. I am posting this to mark my place. If someone else wants to take a crack at it, they are welcome.

  // Add fields.
  field_attach_form('file', $file, $form['options'], $form_state);
greg.1.anderson’s picture

Status: Needs review » Needs work

#31 is essentially the same as the approach I was taking in #13 and #14.

aaron’s picture

Status: Needs work » Needs review

So you,re right. I am sorry that I didn't see that earlier.

greg.1.anderson’s picture

No problem. This issue should be 'needs work', though, as the ability to save the displayed fields is necessary for this patch to be functional. Unfortunately, it is not a simple matter to implement; we need some Javascript code to invoke a service function to invoke a hook that could record state, as that does not yet exist in the current implementation.

greg.1.anderson’s picture

Status: Needs review » Needs work

Forgot to set status above.

aaron’s picture

This gets a little bit further along, but still needs work. I am posting this as a placeholder for myself.

aaron’s picture

This goes a little further. I would love to get some eyes on this, particularly on submitting the form values.

aaron’s picture

Status: Needs work » Needs review
FileSize
3.57 KB

Here is a patch that works. Its a little rough. Not yet handling multiple field values.

aaron’s picture

Thanks chertzog for the help.

aaron’s picture

This patch gets us further, and should handle multiple field values just fine. However, I have additionally removed the title and alt fields from the image override, and now the title and alt default fields do not work.

jwilson3’s picture

+++ b/includes/media.filter.incundefined
@@ -305,14 +305,29 @@ function media_token_to_markup($match, $wysiwyg = FALSE) {
+    ¶
+    // Grab the fields from the file. ¶
...
+ ¶

Watch out for trailing whitespace.

+++ b/includes/media.filter.incundefined
@@ -305,14 +305,29 @@ function media_token_to_markup($match, $wysiwyg = FALSE) {
+      if(substr($field_name, 0, 6) == 'field_'){
+        $rest_of_string = substr($field_name, strpos($field_name, '[') + 1);
+        $lang = substr($rest_of_string, 0, strpos($rest_of_string, ']'));
+        $rest_of_string = substr($rest_of_string, strpos($rest_of_string, '[') + 1);
+        $delta = substr($rest_of_string, 0, strpos($rest_of_string, ']'));
+        $rest_of_string = substr($rest_of_string, strpos($rest_of_string, '[') + 1);
+        $value_string = substr($rest_of_string, 0, strpos($rest_of_string, ']'));

This is really cumbersome logic, could it be rewritten as something like the following (untested code):

if (strpos($field_name, 'field_') === 0) {
  $components = explode('[', str_replace(']', '', $field_name));
  $lang = $components[1];
  $delta = $components[2];
  $value = $components[3];
aaron’s picture

Thank you. Here is a patch with your suggested changes. I still need to figure out about the alt and title attributes.

jwilson3’s picture

+++ b/includes/media.filter.incundefined
@@ -305,14 +305,27 @@ function media_token_to_markup($match, $wysiwyg = FALSE) {
+    ¶
+    // Grab the fields from the file. ¶
...
+ ¶

+++ b/media.moduleundefined
@@ -1072,6 +1073,13 @@ function media_file_displays_alter(&$displays, $file, $view_mode) {
+  // Override the fields of the file when requested by the WYSIWYG. ¶

Still have the trailing whitespaces. If you can, set up your code editor to highlight trailing whitespaces, its really helpful.

+++ b/includes/media.filter.incundefined
@@ -305,14 +305,27 @@ function media_token_to_markup($match, $wysiwyg = FALSE) {
+    foreach($tag_info['fields'] as $field_name => $value){
+      if (strpos($field_name, 'field_') === 0) {
+        $components = explode('[', str_replace(']', '', $field_name));
+        $lang = $components[1];
+        $delta = $components[2];
+        $value_string = $components[3];
+        $fields[$components[0]][$lang][$delta][$value_string] = $value;
+      }
+    }

This can still be cleaned up further, for readability:


// Version 1
foreach($tag_info['fields'] as $field_name => $field_value){
  if (strpos($field_name, 'field_') === 0) {
    // One liner.
    list($name, $lang, $delta, $value) = explode('[', str_replace(']', '', $field_name));
    $fields[$value][$lang][$delta][$value] = $field_value;
  }
}

// Version 2
foreach($tag_info['fields'] as $field_name => $field_value){
  if (strpos($field_name, 'field_') === 0) {
    // Or in 5 lines, for clarity.
    $components = explode('[', str_replace(']', '', $field_name));
    $name = $components[0];
    $lang = $components[1];
    $delta = $components[2];
    $value = $components[3];
    $fields[$value][$lang][$delta][$value] = $field_value;
  }
}
kccmcck’s picture

The patch in #42 worked for me. Now what about using the same display settings as set here: /admin/structure/file-types/manage/image/display ?

aaron’s picture

@Kathryn531: the display settings should work just fine with this patch. If you are not seeing that behavior, perhaps you should open another issue, please.

@jwilson3: this patch rolls in your suggested changes. Thanks, and I will keep pounding at it to get the alt and title attributes working again. I have configured netbeans to erase trailing white space before saving.

azinck’s picture

@Kathryn531: The display settings will not be reflected in the WYSIWYG context itself, only when viewing the text area as fully rendered through the input filters. In the WYSIWYG itself Media is limited by the fact that the macro must be replaced by an image as a placeholder and can't be replaced by arbitrary HTML.

aaron’s picture

The alt and title attributes are not working for fields either, I just realized. That means that it is an issue separate from this. I will take a look and see what might be happening. Of course, this patch does revert the behavior of those attributes from within the wysiwyg but that was a hack anyway.

aaron’s picture

Eureka! I have found it! This patch should be good to go, and you can ignore my comment in #47, as that was user error.

aaron’s picture

Issue tags: +7.x-2.0 alpha blocker

7.x-2.0 alpha blocker

aaron’s picture

This patch fixes an error when there is yet no file selected. I also added some inline comments.

aaron’s picture

I am trying to check field permissions of the user who added field modifications in media_token_to_markup(). Does anyone have an idea about how to best go about doing that?

aaron’s picture

I am checking actually against the "edit" access. How can I check that for the user who has actually edited/overridden the field value in the phase that we are in?

azinck’s picture

@aaron: I haven't yet looked at the work you've done on this patch. I know the module currently allows you to override the alt/title fields in the WYSIWYG (not edit their true values in the db, but override them just in this context). Is your intent to check field permissions at rendering time to determine if we should honor the user's in-WYSIWYG-context-only field override? I'd argue this level of permission-checking isn't necessary (nor is it really possible, I think) since the user isn't changing the actual saved values for those fields in the db.

If further security around that really is needed then I'd propose adding something like "allow user to override file fields on embed" to the input filter settings then ensure those settings get passed through to media_token_to_markup().

Apologies if I'm misunderstanding you. And thanks so much for your work here! I'm looking forward to checking it out and I suspect we'll be using it on a couple projects even this month.

Grayside’s picture

I am looking for global copyright and node-specific caption overrides. So if overriding is an option, it's a field-level one IMO.

Grayside’s picture

Okay, playing with this patch on the tip of 7.x-2.x.

  • Core functionality to override any field works.
  • Pressing the media button without selecting the images, the tabbed structure separating library, upload, etc is broken. This seems to be the case without the patch.
  • If I were to update the central data on the entity, there is no way to instruct the image to respect the central authority without hand-editing code.
aaron’s picture

Pressing the media button without selecting the images, the tabbed structure separating library, upload, etc is broken. This seems to be the case without the patch.

This requires its own issue.

If I were to update the central data on the entity, there is no way to instruct the image to respect the central authority without hand-editing code.

Yes, that exposes a flaw in my design. Thanks for pointing that out, I will work on that.

aaron’s picture

@azinck: I hadn't thought of it that way. It would certainly make things much easier to implement. Considering the wall that I'm up against with this code, I might just let it stand for now, and we can hash it out in another issue.

jpstrikesback’s picture

If I were to update the central data on the entity, there is no way to instruct the image to respect the central authority without hand-editing code.

@Grayside Actually this is by design if I understand what you want to do correctly. The Alt & Title should contain what is in its field if nothing is set already in a media embed tag via WYSIWYG, but when it is overridden on say 5 different embeds (for perhaps: different view modes/ presets) then there is no storage mechanism that I know of other than the body field inside the media embed tag. In that case if we want to be able to reset those stored (in body field) values then we need something that looks at file usage and edits a body field, at that point we'd need to determine if that should create a new revision, to me it gets somewhat sticky, perhaps it is necessary for some tho. Let me know if I'm missing something.

32i’s picture

after applying patch I've noticed following errors:
Notice: Undefined offset: 3 in media_token_to_markup() (line 315 of media/includes/media.filter.inc).
Notice: Undefined offset: 2 in media_token_to_markup() (line 315 of media/includes/media.filter.inc).

if (strpos($field_name, 'field_') === 0) {
list($name, $lang, $delta, $value) = explode('[', str_replace(']', '', $field_name));
$fields[$name][$lang][$delta][$value] = $field_value;
}

It looks like that sometime there's no delta's and values in the fields - it really depends on field type. Is this piece really needed?

32i’s picture

I've updated patch with changes that should fix notices mentioned above, please find attached.

aaron’s picture

@32i: This looks good. Thanks for doing that. I wonder if there will always be a second element in that array? I don't know the internals of the field api well enough.

32i’s picture

@aaron yep, it's always there, it's language definition that either und or language code in case if field is translated. third and forth element may change depending on field type, but second - no.

sarvab’s picture

I've tried the patch from #60 which works great for the FIELDS attached to a file, however I also need to be able to just change the name of the file which doesn't come through with that patch and I believe is pretty important as well.

Maybe not so for images, but for instance uploading documents will default to just rendering a link of the file name. Changing the name allows me to edit what that link says.

For my use-case overriding the name in the WYSIWYG token data is not critical, so I've just put together a little patch that just lets me get to the full Edit File form directly from the Display Format form and kinda integrates with the modal. I know this likely isn't what should go here in the long run, but just in case it helps anyone else as well here is the patch.

aaron’s picture

@sarvab: This is an interesting approach. I think that it might not work for the scope of this issue, because we are modifying or overriding the fields, whereas will likely affect the database values for the file. However, it gets an A for simplicity. And I think that it should go in, and it also exposes the fact that we need to be clear to the user whether they are editing the file, or simply overriding its values.

aaron’s picture

Chasing head.

John Pitcairn’s picture

Status: Needs review » Needs work

When selecting an existing image placeholder and clicking the media button to edit field values, the fields are always populated by the default values from the file entity. Users will expect to see the values from the embedded media tag.

aaron’s picture

Status: Needs work » Needs review

That is a valid concern. However, I note that it is a current bug, as we can also not change the display mode when editing it in the wysiwyg, as noted at #2003810: When editing a file in the WYSIWYG, the "Display as" view mode selection does not inherit the default value.

I am not certain how to easily do this at this time. I will take a look and see if I can figure them both out, but meanwhile I don't believe that this should hold up this patch.

aaron’s picture

This patch addresses that concern. We also fixed the issue in #2003810: When editing a file in the WYSIWYG, the "Display as" view mode selection does not inherit the default value so I'm going to close that issue as a duplicate.

aaron’s picture

It helps to include the patch!

bneil’s picture

Status: Needs review » Needs work

I've tested the patch in #69. Overriding the alt and title fields results in the expected overridden values in the token and rendered markup. However, if I create a new test field (in this instance, "caption") and try to override it, I only get the default value of the field in the token markup and subsequently the rendered markup.

I also continue to have the issue identified in #2003810: When editing a file in the WYSIWYG, the "Display as" view mode selection does not inherit the default value as well as the fields are still populated by the default values from the file entity as identified in #66 when I try to edit an already embedded image with overridden fields.

aaron’s picture

Status: Needs work » Needs review

I am not certain why it doesn't work for you. Everything works for me after applying the patch to the latest dev release.

aaron’s picture

Assigned: becw » aaron

I have moved the alt and title descriptions that were added at #1881152: Browser display cleanup into the file entity module so that they are available everywhere, and not just during the WYSIWYG override, at #2012868: A better description for the alt and title attributes of images.

Dave Reid’s picture

Moving to a beta blocker. I don't think feature requests should hold up a long-awaited alpha release.

Dave Reid’s picture

Issue tags: -7.x-2.0 beta blocker
Dave Reid’s picture

Issue tags: +7.x-2.0 beta blocker
aaron’s picture

Normally I would agree with you, however, in this case, I believe that this issue should be committed before the alpha release. Because the feature that went in at #1881152: Browser display cleanup is fundamentally broken, and I would like to get this in before even the next unstable release so that we don't have to worry about supporting it.

Basically, in that issue we added alt and title overrides to images. However, it was done incorrectly, I would argue, by not actually overriding the field values, but by simply attaching the values willy-nilly to the content. The method that I introduce here overrides the actual field values, including any other fields that have been defined for the file type. Because of how the original issue was implemented, it would be difficult at best to backwards support any overrides that have already been put in place. I would think it to be preferable, assuming that we plan to implement this in the fashion that I have created, to not support the broken overrides.

jpstrikesback’s picture

I'd like to bring up again (as in #58 and in #1307054: Accessibility - Media browser image alt and title fields #114 & #115) that changing the alt/title on one media tag should not change it across the board, or at least this should be configurable. If it is changed across the board a stock image cannot be used in multiple places with different alt/title while maintaining a default set on the field when uploaded (or modified on the file edit entity ui)

The behaviour you are talking about is certainly important but it should be configurable, i.e. "Set all instances to use the new Alt/Title text" IMO.

Open to being shot down here but that is how it was first committed and changing that behaviour would be a regression to some.

Here is an example. A pet site has a bunch of pictures of siamese cats, when bloggers blog on said site they can upload new ones or use existing pictures of siamese cats, if they change the alt/title across the board they will likely cause problems with other bloggers interpretations of said cat image. :)

jpstrikesback’s picture

Also, I have no clue how the tag changed there...please feel free to set back

azinck’s picture

@jpstrikesback: I still have yet to test this patch (really need to get around to it), but my assumption all along has been that aaron's attempting to provide exactly the functionality you're talking about: the ability to override arbitrary field entity fields on a per-usage basis. If my assumption is wrong then I share your concerns.

jpstrikesback’s picture

Yep in #76 we see

The method that I introduce here overrides the actual field values

This is a regression unless made configurable at time of edit IMO

gaele’s picture

as requested in #78

bneil’s picture

It is also my understanding that the scope of this issue is about overriding fields (alt, title, maybe even a caption?) on a per instance basis, not changing the global value.

If that's the scope though, I'm also concerned about the user experience for the content editor when presented with these fields on the per instance basis, making it clear that they will be overriding the fields on this one use case. Furthermore, I'm wondering if this is going to be configurable, so we limit what fields are allowed to be overridable. For example, if there's a term reference field on files, I probably don't want to have the user override that on a per instance basis.

bneil’s picture

Did not mean to change tags.

aaron’s picture

The behavior of this patch is per described at #77. However it is not currently configurable as suggested at #82.

John Pitcairn’s picture

I think if you are going to do this and there are fields on the file, they should all be overridable for that wysiwyg instance. If you don't want certain users to do that for certain fields, then you need to use field access permissions (Content Access module, etc), and wysiwyg should respect that.

Note I don't really want to see this ability at all in WYSIWYG unless it is possible to do exactly the same thing for ordinary file field instances. Otherwise that's a major WTF for users who may be adding images via node fields and also adding images in WYSIWYG - wysiwyg use would be a local attribute override, the other is completely global. See #2000278: Support per-field and per-revision values for the file entity's fields.

aaron’s picture

it respects field access permissions.

bneil’s picture

FileSize
66 KB
58.15 KB

I think if you are going to do this and there are fields on the file, they should all be overridable for that wysiwyg instance. If you don't want certain users to do that for certain fields, then you need to use field access permissions (Content Access module, etc), and wysiwyg should respect that.

I'm not sure field access is going to solve my example, considering I still would want an editor to be able to edit the global value of those fields from admin/content/file.

A more in depth example would be providing the following fields on an image: alt, title, caption, attribution, and a term reference. Ideally, I'd still want an author to be able to edit and configure the global values of all of those fields on upload, as well as from admin/content/file, so I wouldn't do any field-level access permission.

But, since the attribution and term reference fields don't make sense to be overridable in my use case, I wouldn't want them shown in the same form where the other fields can be overridden.

I hope I'm not hijacking this issue, and if I am, please let me know and I can post this all in a separate issue. But having an interface, when selecting a file, where the content editor can edit predetermined "override-able" fields for that usage, or edit the global values would be really useful. I've attached a quick wireframe of what I'm thinking.
The override fields interface
The global fields interface

Using my example above, I could then either use the default alt/title text or override that, use a default caption or override it, or globally add a term to the file or change it's attribution.

Note I don't really want to see this ability at all in WYSIWYG unless it is possible to do exactly the same thing for ordinary file field instances.

I agree, this should be also possible for file fields.

greg.1.anderson’s picture

#87 is brilliant. It would be fantastic if the dialog could be made to work this way.

aaron’s picture

Status: Needs review » Needs work
FileSize
795 bytes

Here is an initial attempt to add a wywiwyg override value to fields. Ignore the dpm statement. More to come.

aaron’s picture

Status: Needs work » Needs review
FileSize
8.88 KB

This one allows us to override only the fields which we have checked the wysiwyg override button in the fields settings.

azinck’s picture

I'm loving the direction this is going. Thanks for your work, aaron.

HyperGlide’s picture

Aaron -- nice work. Can you advise on the status of the patches?

This issue is approaching the (3) year mark.

jpstrikesback’s picture

Thanks Aaron!! Will be testing on simplytest.me soon!

aaron’s picture

@HyperGlide, the patch should be good to go. I am just waiting on an RTBC, and am anxious to see this issue closed, as we are now just over the three-year mark.

hughtebby’s picture

Nice work ! Just tested with several fields and file types, and everything works fine.

Edit : actually, if I'm not mistaken this patch actually removes the feature of being able to override the values for each instance. While I've actually never needed that feature (and usually my customers have difficulties understanding it) it would probably be useful... (and would it be a problem when updating the module ?)

slashrsm’s picture

Status: Needs review » Reviewed & tested by the community

Looks nice. I tested it with alt and title and also another text field. Everything worked as expected. I think it is ready to commit.

Highlighted "Add media" button can be easily overseen. It would be nice UX improvement to make this much more visible. We can do this in a followup, since this is a blocker and lots of users most likely expect this feature to be added ASAP.

@hughtebby: It looks like this patch keeps this feature intact. Evern more. It now allows you to override all fields, not only alt and title.

aaron’s picture

Status: Reviewed & tested by the community » Fixed

Committed to http://drupalcode.org/project/media.git/commit/bf3cde3. Thanks for the hard work, everyone!

Status: Fixed » Closed (fixed)
Issue tags: -7.x-2.0 alpha blocker

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

Anonymous’s picture

Issue summary: View changes

Added initial issue summary.

justinlevi’s picture

Component: WYSIWYG integration » Code
Issue summary: View changes

Realize this is a very old thread, but wondering what ever happened to this functionality? I'm not seeing it with the latest dev branch from the media module or in media_ckeditor.