Wysiwyg integration is broken

Same problem here, but not sure the problem lies with Wysiwyg.
It only started when I upgraded a couple of modules and the Drupal version. Media was one of them.

I am getting the same issue with altering an images width/height, since updating to alpha2.
As this is something clients needs to use, I have put together a simple/hacky patch that seems to fix my issue. (Still some bugs, some probably caused by my changes)

The call to Drupal.media.filter.allowed_attributes in the extract_file_info() method, was missing the (), so all attributes where being striped when extracting the file_info from an element.

The main change to the file is checking if the marco code has changed, and if so replacing it within Drupal.settings.tagmap.

This was just a quick fix that will allow non-coders (clients) to continue to use the media module. I wouldn't want to use this in a production environment at the moment, as whenever I used WYSIWYGs 'Disable rich-text' option, then enabled it again, it completely killed all the media elements on the page.

Ran into issues with file_aliases and filefield path modules with this patch. Just disabled them for now.
Cannot use object of type stdClass as array in *sites/all/modules/file_aliases/modules/filefield_paths.inc on line 33

We have been having issues setting the image alignment using the wysiwyg module with CKEditor (3.6.6.1 & 4.0.3).

Implementing this patch on the latest dev version now simply renders embedded images with the string 'false' for me. After playing around with it a bit, I realized that content.substr(startPos, endPos) fundamentally doesn't work, as the second argument should be the length of the string, not the end index. Even correcting that, it only works when the image is first embedded into the content; subsequent saves convert the image into the string 'false' again. When it does work, element attributes are retained, which addresses the primary issue at hand.

I modified the patch from #7 to use string length as argument 2 of substr. Also, in the event that updateMacro is false, don't replace the macro with a string 'false', just to be safe. Updated a few other code standards issues.

This patch is far from production quality. I'm still getting images stripped when toggling between disable and enable rich text. Also, if you drag an image around in the content itself, the image gets stripped out.

I am now wondering if this should be approached slightly differently. Rather than storing the macros and assuming they haven't changed between page load and node save, should the call to replacePlaceholderWithToken() grab the entire content, parse out the media in their latest state, and then store those macros? If I have time tomorrow, I'll throw together that approach to see if it resolves some of the media manipulation inconsistencies between page load and node save.

Tested the patch from #10.

+            if (!!updateMacro && updateMacro != macro) {
+                Drupal.settings.tagmap[updateMacro] = updateElement.html();

Seems that the tagmap gets updated by an empty string due to the html() method.
I think this is because an image has no "innerHtml" and to use html() on an image returns an empty string..
I think we can change it to

Drupal.settings.tagmap[updateMacro] = Drupal.media.filter.outerHTML(updateElement);

and use the existing outerHTML method.

I decided to play with the concept of re-building the tagmap when replacing tokens with placeholders to ensure every and all changes to the markup were registered into the macro system. This patch solves a few remaining issues that previous patches don't address, namely the ability to drag an image into a different position within the markup. From the code side, it also feels more concise.

The only hesitation I have with this patch is the fact that I needed to replace all instances of ' />' with '>' in the content before attempting the replace() methods, as the image tags within the content string are using the ' />' xhtml markup, but jQuery converts these elements to use the '>' html closure. I'm not sure what this implies for cross-browser support, but I'm relatively sure it's a clean workaround.

One other thing to note is that $.each uses a zero-index iterator, but our media wrappers start at one. I propose we shift to using zero-index wrappers across the board due to the coding practice of iterators starting at 0, but that's a different discussion.

Thoughts on this approach?

I have yet to test the patch as I'm having a few issues getting the start comment of the media filter token to appear. However, we've got quite a few sites that were using media before alt/title text was working, so editors were embedding an image, right clicking on it, and clicking the CKEditor image properties and defining alt/title there, which worked. Now after incrementally upgrading (to alpha1) those values are no longer available. I'm hoping this issue can help resolve that as well. New sites we deploy are using the alt/title from the Add media button, but we've unfortunately got several that we still need to support any alt/title text that was added via the CKeditor image properties dialog.

This patch does not work for tinyMCE. After resizing and saving, the image no longer appears in the content. However, it does at least seem to save the settings when you go back to edit it.

Patch #12 plus the addition of parentheses to Drupal.media.filter.allowed_attributes() on line 190 does seem to keep the style attribute intact, and this information (including the float) is included in the media tag string returned by the create_macro method.

The style attribute survives the WYSIWYG editor being toggled, and is also saved into the media "[[...]]" tag :)

But the node is still rendered without the style attribute :(

Patch #12 works with a single media element. When testing with multiple media elements, only the first element saved as a macro tag. The remaining elements were saved as the html placeholders.

Example of 2nd saved media image.
<img alt="" class="media-element file-default" data-file_info="%7B%22fid%22:%2219%22,%22view_mode%22:%22default%22,%22fields%22:%7B%22format%22:%22default%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_file_caption%5Bund%5D%5B0%5D%5Bvalue%5D%22:%22%22%7D,%22type%22:%22media%22%7D" height="100" src="/sites/default/files/styles/media_thumbnail/public/family.jpg?itok=8gjJsufU" title="" width="100" />

WYSIWYG module: 2.2
Editor: CKEditor 3.6.6

Edit: Can confirm #19's image linking issue and that patch #12 fixes it. Before it was replacing the custom link and media image with the text 'false'.

Patch #12 fixed a related issue for me when trying to wrap images with custom links via CKEditor.

Previous the a link would get dropped on saving or disabling the CKEditor.

Thanks
Tom

From what I can tell, it sounds like my patch from #12 was on the right track, but was missing a few important pieces.

I've added a bit to the patch:

A) Included the Drupal.media.filter.allowed_attributes() on line 190.

B) The multiple media element issue from #18 was related to the way the xhtml closing tags were being replaced. Updated to a global replace to ensure proper string matching and replacing.

C) Attributes on the rendering end of things weren't in the right spot. The latest dev build puts the 'attributes' array into $element['#attributes'], which would work on most render arrays, however, theme_image_formatter requires the 'attributes' to be set on $item['attributes'], which in this context is in $element['#item']['attributes']. Should be fixed in this patch, allowing the attributes to be properly set on the element.

D) I have not tested tinyMCE with this patch; only CKEditor. I'm wondering if the rendering update from (C) also resolves the issue for tinyMCE?

I'm bumping the priority up on this one to major, because this functionality as it currently exists is unusable and is a relatively large feature of the media module.

Let me know if this one works better for you. Thanks for all your feedback so far!

Setting the version 7.x-2.x-dev. Rerolled; Take 2.

Using 7.2-2.x-dev and patch #22 works nicely here, even on pages with multiple images, and thus fixes the problems with image attributes missing since alpha2.

Thank you kaidjohnson for your work on this!

Since upgrading to media 7.x-2.0-alpha2, I have not been able to resize images.

I upgraded to media 7.x-2.x-dev (2013-Sep-17 ) and applied patch at #22.

I'm also using wysiwyg 7.x-2.2 with CKEditor 3.6.6.1.7696 and Drupal 7.23.

Now I can resize images, but only after I save the node twice. After the first edit/save, when you view the image is not resized. On the second edit/save (with no changes), then view the image is resized.

@ellen.davis - I can't reproduce the issue in #24 using CKeditor 3.6.6.1.7696 or 4.0.3. After your first save, assuming the image isn't resized as expected, what happens if you simply flush your caches? Also, what browser (and version and operating system) are you using?

I was testing in Firefox on a Mac. Flushing the caches does nothing to the image.

I did more testing on various browsers.

Firefox 17.0.8 on Windows 7 – first save image is still original size. Second save image is resized.

Internet Explorer 10 on Windows 7 – first save image is resized. Works here!

Chrome 29 on Macintosh 10.6.8 – never gets resized

Safari 5.1.9 on Macintosh 10.6.8 – never gets resized

Firefox 23.0.1 on Macintosh 10.6.8 - first save image is still original size. Second save image is resized.

Thanks @ellen.davis for the feedback. I'm still not able to reproduce the issue you're seeing.

I have tested with success (image can be resized/re-aligned/vspace/hspace etc) on new nodes and changes are reflected after the first save as expected. On existing nodes changes are also reflected with one save as expected.

I have tested on:
* Ubuntu 13.04 / Chrome 29
* Ubuntu 13.04 / Firefox 22
* Ubuntu 12.04 / Chrome 29
* Ubuntu 12.04 / Firefox 22
* OSX 10.7.5 / Chrome 29
* OSX 10.7.5 / Firefox 23
* OSX 10.7.5 / Firefox 24
* OSX 10.7.5 / Safari 6.0.5

It's interesting that it did work for you on IE10 -- I haven't had the energy to spin up my virtualboxes to do a full round of IE testing.

@ellen.davis - what process are you using to resize your images? Do you have any other wysiwyg/ckeditor/media/file_entity related modules enabled? Are any javascript errors being thrown in your console(s), especially on those tests where you're not able to resize the image at all? Could you test using the media_dev profile and let me know if you experience the same results?

Can anyone else reproduce the issue @ellen.davis is experiencing?

* Edited -- Added successful Safari test to the list.

Tested this with TinyMCE v3.5.8 and Safari v6.0.5, on the current -dev release of File_Entity and Media, and it worked nicely.

I recommend using the Devel module to help test the values that are saved, just to see if the code includes the desired attributes, e.g.:

[[{"fid":"9798","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":""},"type":"media","attributes":{"height":362,"width":499,"style":"float: right;","class":"media-element file-default"}}]]

Marked #2099145: Unable to add a link to embedded images as a duplicate, the patch in #22 appears to have resolved it.

With kaidjohnson's review in #27 and my own experiences, am marking this as RTBC. Good work everyone!

I seem to be having similar issues to ellen.davis.

Im using the latest dev with CKEditor 4.2.0 and have applied patch 22 but i cant seem to change any attributes in chrome or safari on OSX 10.8.5. Firefox seems to work better but not all the time.

I cant see any console errors and it all looks fine up until i save it, then it just doesn't apply the styles.

Using a clean install of the Media Dev profile. WYSIWYG API + CKEditor 3.6.6.1.7696.

Tested in OSX 10.7.5
Chrome 29.0.1547.76
Safari 6.0.5
FireFox 24

Before Applying Patch:
<p>[[{"fid":"1","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"woo alt","field_file_image_title_text[und][0][value]":"woo title","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{}}]]</p>

After applying patch
<p>[[{"fid":"1","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"woo alt","field_file_image_title_text[und][0][value]":"woo title","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{"height":1350,"width":1180,"class":"media-element file-default"}}]]</p>

So the "default" attributes (based on view mode?) do come through.

Next, I tried editing properties via the CKEditor Image Properties dialog, where I scaled the image down, and added a float right.

This was reflected in the WYSIWYG preview and in the token markup as "style":
<p>[[{"fid":"1","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"woo alt","field_file_image_title_text[und][0][value]":"woo title","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{"height":1350,"width":1180,"style":"width: 399px; height: 456px; float: right;","class":"media-element file-default"}}]]</p>

After saving, I get the following rendered markup:
<p><!--MEDIA-WRAPPER-START-2--><img height="1350" width="1180" style="width: 423px; height: 484px; float: right;" class="media-element file-default" typeof="foaf:Image" src="http://media.dev:8888/sites/default/files/Druplicon.large_0.png" alt=""><!--MEDIA-WRAPPER-END-2--></p>
Which no longer includes the alt text from the file entity, but it's value is part of the token. Looking at the alt/title stuff a bit further, if I save global values to these fields from the file entity's edit form outside of the media module, those values render just fine. However, if I set any "override" on a per usage basis, they do not update.

And back in the token markup: (Note, this is the first time I've seen the <!--MEDIA-WRAPPER-START/END-X--> part of the token)
<p><!--MEDIA-WRAPPER-START-2-->[[{"fid":"1","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"woo alt","field_file_image_title_text[und][0][value]":"woo title","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{"height":1350,"width":1180,"style":"width: 423px; height: 484px; float: right;","class":"media-element file-default"}}]]<!--MEDIA-WRAPPER-END-2--></p>

So, in summary, it looks like any per-instance field overrides are not being passed in, at least in CKEditor 3.6. And I'm unsure when I'm supposed to see the html comment wrapper.

I will also provide testing with a previous site upgraded to file_entity/media dev and this patch.

@Kline: What version of ckeditor are you using?

I setup media_dev-7.x-2.x-dev to do some testing. THe one change I made was to disable generic file at admin/structure/file-types/manage/image/file-display.

media_dev-7.x-2.x-dev without patch #22:
Tested with IE 10 on Windows 7 and Firefox 24 on Macintosh.
Inserted image, set width, height, border, hpsace, vspace, alignment. Saved. I viewed the saved body contents using devel.

[[{"fid":"8","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{}}]]

If I edit the node and save again (making changes or not), the image is still saved as media macro but with missing attributes.

[[{"fid":"8","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{}}]]

Applied patch in #22.
Test with Firefox 24 on Macintosh (with patch).
Inserted image, set width, height, border, hpsace, vspace, alignment, saves as media macro with attributes.

[[{"fid":"9","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{"height":720,"width":960,"style":"height: 225px; width: 300px; border-width: 5px; border-style: solid; margin: 10px;","class":"media-element file-default"}}]]

But, when I edit and make no changes, then Save , the inserted image is saved as img tag.

<img alt="" class="media-element file-default" data-file_info="%7B%22fid%22:%229%22,%22view_mode%22:%22default%22,%22fields%22:%7B%22format%22:%22default%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,%22media_description%5Bund%5D%5B0%5D%5Bvalue%5D%22:%22%22,%22field_tags%5Bund%5D%22:%22%22%7D,%22type%22:%22media%22%7D" height="720" src="http://myhostname/media_dev-7.x-2.x-dev/sites/default/files/ice3.jpg" style="height: 225px; width: 300px; border-width: 5px; border-style: solid; margin: 10px;" typeof="foaf:Image" width="960">

When I edit and make changes, the inserted image is again saved as media macro.

[[{"fid":"9","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{"height":720,"width":960,"style":"height: 225px; width: 300px; margin: 10px;","class":"media-element file-default"}}]]

IE 10 on Windows 7 (with patch)

Creating a new node , and edit/save (with changes or no) saves as img tag with attributes set.

So look like the patch does save the attributes. But is causes the image to be saved using img instead of media macro in some cases.

I'm seeing an issue where if I have an image does not need to be resized by an image style it will not apply the attributes (so if I just choose the "Default" display). BUT if I apply an style to it (make it a teaser, for example) my attributes stay.

I tracked down the difference to how the two elements are generated, but am not sure where to go from there.

Images that are resized seem to call in file_entity_file_formatter_file_field_view, which populates $element['#item']. Images that aren't resized call file_entity_file_formatter_file_image_view, which does not populate an $element['#item].

So this change in patch 22:

-  if (!isset($element['#attributes']) && isset($settings['attributes'])) {
-    $element['#attributes'] = $settings['attributes'];
+  if (!isset($element['#item']['attributes']) && isset($settings['attributes'])) {
+    $element['#item']['attributes'] = $settings['attributes'];

Is stripping out attributes for unresized images.

I recommend doing this instead:

-  if (!isset($element['#attributes']) && isset($settings['attributes'])) {
-    $element['#attributes'] = $settings['attributes'];
+  if (!isset($element['#item']['attributes']) && isset($settings['attributes'])) {
+    $element['#item']['attributes'] = $element['#attributes'] = $settings['attributes'];

Attached is patch 22 with that change applied.

The patch in #38 didn't work for me. I've reworked it and included some other changes I had been previously working on. I have only tested against the ckeditor 4.x.

Here's an updated patch against the latest dev. This patch successfully solves the following problems for me:

• Being able to update alt and title text using the media editor for previously inserted images
• Being able to update the view mode using the media editor for previously inserted images
• Being able to update various image properties using the ckeditor image properties dialog for previously inserted images
• Being able to update previously inserted images using the media editor without destroying ckeditor dialog inserted image properties

It does not resolve the issue where the tagmap gets corrupts when you disable rich-text, manually edit a media tag, and then re-enable rich text. I think this would be resolved if we also rebuilt the tagmap from macros into markup in replaceTokenWithPlaceholder(), but I didn't get to that. If we do that, should we remove the tagmap altogether? Is there some other purpose it would still be serving?

Also note that I wasn't able to figure out whether stripping the wrapper tags had any adverse effects.

@rocket_nova, your patch won't apply to the 7.x-2.x branch.

@mariacha1, thanks for that update! That idea got me digging a little deeper into what was going on right there.

Basically, theme_image_formatter() uses a slightly different structure than other theming functions, so I've updated the way all of that attributes logic is handled to ensure theme_image_formatter() gets everything it is looking for.

Also, per #41, this latest patch allows for direct editing of the macro itself via the plain text format.

This patch did raise a handful of deeper questions/issues (of which I believe should be addressed in other/new issue queues):
1) When the title field and alt field are set (the real fields, not the WYSIWYG attributes), should they override the title/alt attributes in the WYSIWYG editor? Should they somehow know about each other and update accordingly so they are always in sync?

2) A known bug with this patch is that using the Media popup to change field values will strip out all formatting already in place via the WYSIWYG. It appears that this is because media elements are considered 'new' after submitting that overlay so the previous element is destroyed and re-built. I believe this issue is technically out of scope of the current thread; should this patch get picked up by the project, we should create a new thread for exactly this issue to address it.

3) By now building macros via the content and building the content via the macros in the content (i.e. not in the tagmap), I'm wondering what the need for the

placeholders is? I was able to strip that out completely with no side effects, but didn't want to include it in this patch because I thought it was out of scope or I might be overlooking something important with them.

4) Similarly, the tagmap feels less important, however it is much more deeply embedded in other functions related to these media elements, so, to answer the question in #41 -- Yes, I believe the tagmap could be removed, but no, I don't believe removing it is a good idea at this time. It could be a good topic for another thread if/when this thread gets resolved.

5) The primary reason title/alt tags weren't rendering on the image was because they were not whitelisted by default! I have added them both to the whitelist. I get the feeling there was a reason those attributes weren't whitelisted, but without them, alt/title tags will never render on an image.

That's all I got. Let me know how this works for you and if it solves some issues above. (namely those experienced by @ellen.davis outlined in #36).

Paris committed https://drupal.org/node/2104839 ( looks like it's the patch from #2104839: extract_file_info does not call allowed_attributes() properly ) so reroll:

I am using media 2.x, wysiwyg-dev, ckeditor 4.x and applied the patch from #44.
When editing a node (with multiple media images in it from before the patch) all images are missing. When viewing the node I can still see the images.
I don't see any JS errors. When switching the editor off I see these empty tags: <!--MEDIA-WRAPPER-START-0--><!--MEDIA-WRAPPER-END-0-->, I also see those when I switch to source mode in the editor.

When I add a new image it looks fine at first, but when I disable the editor, the image is replaced by <p>false</p>. I also get no errors there.

@Yaron Tal - Thanks for the feedback. I'll check it out when I get a chance.

For testing purposes, let's all be sure we're on a standard platform and testing using: http://drupal.org/project/media_dev. This issue is quite complex and seems to have a handful of wildcards in it that we can normalize by at least using the same core install.

Again testing with media-dev distribution. Patched with https://drupal.org/files/allowed_attributes_function_call.patch and https://drupal.org/files/media-missing-attributes-2067063-44.patch

Testing with FF on Mac.

Create an article, using the WYSIWYG to insert an image. View body[und][0][value] contents with devel. Saves as image macro (as expected).
• <p>[[{"fid":"6","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","attributes":{"height":720,"width":960,"class":"media-element file-default"},"tagName":"IMG","src":"http://my.host.name/media_dev-7.x-2.x-dev/sites/default/files/penguins_0.jpg"}]]</p><p></p>

But when edit the same article, make changes, save. Saves now as img instead of macro. Each time you edit/save you get another MEDIA-WRAPPER-START and END.
• <p><!--MEDIA-WRAPPER-START-0--><!--MEDIA-WRAPPER-START-0-->[[{"fid":"6","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":"","media_description[und][0][value]":"","field_tags[und]":""},"type":"media","tagName":"IMG","src":"http://drupaldev.uc.edu/media_dev-7.x-2.x-dev/sites/default/files/penguins_0.jpg","attributes":{"height":720,"width":960,"style":"width: 300px; height: 225px; border-width: 5px; border-style: solid; margin: 10px; float: left;","class":"media-element file-default"}}]]<!--MEDIA-WRAPPER-END-0--><!--MEDIA-WRAPPER-END-0--></p><p>&nbsp;</p>

In debugging, I found an that in

content = content
          .replace(el.outerHTML, macro)
          .replace(startTag, '')
          .replace(endTag, '');

startTag = <!--MEDIA-WRAPPER-START-1--> but content has <!--MEDIA-WRAPPER-START-0--> so replace is not done. same for endTag.

Fix:

In file media.filter.js, in function replaceTokenWithPlaceholder

Replace

content = content.replace(match, Drupal.media.filter.getWrapperStart(i) + markup + Drupal.media.filter.getWrapperEnd(i));

with

var startTag = Drupal.media.filter.getWrapperStart(parseInt(i + 1)),
          endTag = Drupal.media.filter.getWrapperEnd(parseInt(i + 1));
            content = content.replace(match, startTag + markup + endTag);

Another issue when testing on FF on Mac.

When I edit an article and make changes to an image, it is saved as a media macro. But, when I edit an article, make no changes to the image, and save. Then the image is then saved as img tag.

In replacePlaceholderWithToken

content = content
          .replace(el.outerHTML, macro)
          .replace(startTag, '')
          .replace(endTag, '');

In my case, both content and el.outerHTML contain an img tag referencing the same image and attributes. But the elements of the img tag are in slightly different order, to the content.replace(el.outerHTML, macro) fails.

Subscribing

#50 Please use the 'Follow' button in the top right instead of posting a subscribe message.

Thanks for all the feedback, folks! I think this is another round closer.

I believe these issues have been addressed in this patch:

* @Yaron Tal #46 - I am assuming your existing media is stored within the macro tagmap, and the previous patch removed that mechanism. For backwards compatibility and a moderate improvement in efficiency, I'm using the tagmap again to render the macro to markup, BUT now if the tagmap doesn't match the macro, the system rebuilds the element from the macro completely to ensure changes made directly to the macro persist. This also addresses part (4) of comment #42.

* @ellen.davis #48, #49 - I believe the issue was related to the duplicate wrappers. In this latest patch, I have completely removed the wrappers because I can't identify anything that actually depends on them anymore and they seem to be creating needless verbosity to the markup. This addresses part (3) of comment #42.

Keep the feedback coming!

Hi kaidjohnson, thank you for your work on the patch!

Testing with Libraries + CKEditor 3.6.6.1, in Chrome 30.0.1599.69:

After applying the patch, I can make modifications to alignment, size, etc from the CKEditor image dialog in the WYSIWYG and those changes save and are rendered when viewing the node. This also applies for any alt/title text that is applied via that same interface.

However, after making such an override, if I click on an image and click the media add/edit button and change the view mode, None of my overrides stick.

Also, if I change any of the values of the fields on the file entity's edit form, none of my overrides from the CKEditor Image dialog stick, nor do any of the file entity overrides.

However, it looks like the file_entity overrides do come through in the token:
[[{"fid":"6","view_mode":"teaser","fields":{"format":"teaser","field_file_image_alt_text[und][0][value]":"Squash hunters - overridden.","field_file_image_title_text[und][0][value]":"A squash hunter and his minions - overridden.","media_description[und][0][value]":"A good hunt. Here's my override."},"type":"media","tagName":"IMG","src":"http://media.dev/sites/default/files/styles/medium/public/squashhunting.jpg?itok=Rizgtj_z","attributes":{"height":220,"width":154,"style":"line-height: 1.538em;","alt":"Squash hunters","title":"A squash hunter and his hunt.","class":"media-element file-teaser"}}]]

They just don't get rendered.

Setting back to needs work, but maybe these field overrides are out of scope of the original issue.

Testing patch#52 with media_dev distribution on IE 10. Image is saved with <img> tag instead of media macro.

I added a couple alerts in the JS and found that replacePlaceholderWithToken img tag in el.outerHTML does not match the image tag in content. So the replace fails to work properly.

 content = content.replace(el.outerHTML, macro);

I found this ticket on the ckeditor support site. It is marked closed and fixed.
http://dev.ckeditor.com/ticket/1810
Attributes order must not change. One solution for that would be ordering the attributes alphabetically .

So it may be that ckeditor is changing the order of the attributes in the img tag.

I've tested this out on two of our current sites that have media embed tokens that were created before the token markup changed.

When I edit a node with an embedded image and immediately save it, my image is removed. The only difference between revisions is",tagName":"IMG","src":"http://path/to/file.jpg?itok=4lMRPj1R"

Related: #2107465: IE removes MEDIA-WRAPPER-START comment

I think I have resolved at least part of #53; I have mixed feelings about working on that issue here, however. It *is* related to what we're trying to resolve here, but it has its own use-case, so it should probably be addressed under a different issue. That said, I'm curious if this patch at least begins to address the issue...

This patch also attempts to address @ellen.davis's issue in #54. It does appear that there's a little inconsistency with the way jQuery returns the element when matching, so I've added a method that forces sorting of element attributes in alphabetical order, which resolved the issue for me in IE9 and IE10.

@bneil, re #55 -- I have a few sites that also have existing embedded media from before this whole chaos began, but haven't had a chance to look at them carefully with this update. I'm surprised the patch didn't resolve the issue; what browser / OS are you using?

Lastly, re #56 and #2107465: IE removes MEDIA-WRAPPER-START comment, the solution I'm moving forward with eliminates the use of comment wrappers, thus this issue should be moot.

This patch also updates a few non-camel-cased methods to camel-case for code standards.

I just tested this, and it indeed keeps the attributes.
Just throwing to needs review, to let the testbot do some work.

Tests seems to pass. When going to the code, i couldn't find anything which seemed wrong / strange.
Tested with latest stable D7, and latest dev of wysiwyg / file_entity / media, and CKeditor 4.2.1.5010dd4
Can someone point me too the things which are needed tot get this RTBC?

Here's an interdiff of 52-57. I look forward to testing this out later today.

To help people test the patches, we need reviews of:

• IE, Firefox, Chrome, Safari (others would be useful too).
• TinyMCE v3 and v4, ckeditor v 3 and v4 - others might be useful too but these are the two most popular editors.
• As many operating systems as possible as some of the JS & CSS engines behave differently on different operating systems.

Thanks @DamienMckenna
i tested #57 under

• Linux Mint
• Chromium 28 and Firefox 24
• Ckeditor 4

Tested patch #57. Works great. Image attributes are saved. Image is saved using media macro.

Mac 10.6.8
FireFox 24.0 , Safari 5, Chrome 30
CKeditor 3.6

Windows 7
IE 10, FireFox 17
CKeditor 3.6

Thanks @kaidjohnson for all your work on this!

I noticed this breaks media_vimeo module. When i apply this patch, and try to embed a vimeo-video from my library, it's not showing up in the wysywig (nor under 'disable rich text').

UPDATE: As Kaidjohnson mentions in #77, the following issue was with me not having the correct base version of media installed. Sorry for the false negatives, all! Please disregard this bug report.

--------------------------------------------------------------------------------------------------------

Thanks so much kaidjohnson for patch #57! It works almost perfectly...

Linux Ubuntu
FireFox 24.0
TinyMCE 3.5.8

I am pleased to say that #57 solves #2085997: WYSIWYG Image update not working.

Unfortunately, it doesn't seem to solve this problem: #1411340: Resizing images in WYSIWYG broken in media-7.x-1.0-rc3+ & 2.0-unstable3+

When I enter a dimension in the Image editor, it displays properly in the WYSIWYG editor, but the dimensions revert to whatever they are set to by Media on the page.

Here some images to show what I mean.

Adding and resizing the image via the TinyMCE Image button from 100x100px to 10x10px.

Saving and viewing the page:

Re-loading the edit screen shows it still knows the desired width is 10x10px.

The good news is all other attributes (hspace, vspace, border, and styles) are applying correctly. Only dimension seems to not work.

EDIT: Those tests were all done with a default image (no image style applied). Looks like when I try to use a display type (so, apply an image style) the image doesn't show up on the saved page, and the attributes aren't saved.

Updated issue summary

Updated issue summary.

lets write some tests please, we keep doing circles around stuff like that.we constantly break them.
some tests, lets say: put a json tag with overrides in the node body textarea, save, check on node view if overrides apply

Add related issues

i updated the issue summary.
also escalating to critical and beta blocker, since this can/will fix a bunch of bugs and because lets face it, our wysiwyg integration is in really bad shape

d.o ate the tag

A proper issue summary

Tested with Windows 7 Firefox and TinyMCE3. It does not work with that combination, instead I get the markup replaced with an img tag. Additionally, youtube videos do not upload at all to the wysiwyg.

1. +++ b/js/media.filter.js
   @@ -14,36 +14,40 @@
   +      if (!!matches) {
   

   don't we have an extra ! here?

2. +++ b/js/wysiwyg-media.js
   @@ -143,8 +146,8 @@ function ensure_tagmap () {
     * @deprecated
   ...
   -function create_element (html, info) {
   -  return Drupal.media.filter.create_element(html, info);
   +function createElement (html, info) {
   +  return Drupal.media.filter.createElement(html, info);
   
   @@ -155,8 +158,8 @@ function create_element (html, info) {
     *
     * @deprecated
   ...
   -function create_macro (element) {
   -  return Drupal.media.filter.create_macro(element);
   +function createMacro (element) {
   +  return Drupal.media.filter.createMacro(element);
   
   @@ -167,8 +170,8 @@ function create_macro (element) {
     * @deprecated
   ...
   -function extract_file_info (element) {
   -  return Drupal.media.filter.extract_file_info(element);
   +function extractFileInfo (element) {
   +  return Drupal.media.filter.extractFileInfo(element);
   

   is there a reason we are renaming the deprecated wrappers? if they are there they are only for BC, whats the point renaming, we break the BC;)
   either keep as is or remove

Patch in #57 fixes the classes for me (classes now added to image tag where they weren't before) but I'm still having issues with saving alt or title attributes. Also, WYSIWYG image dimensions config seems to never get saved (so similar results to #68).

Mac OS 10.8
Chrome 30
CkEditor 4.0.1.d02739be4b
Media 7.x-2.0-alpha2+10-dev

I'm late to this and am only offering a suggested first step for what might possibly be a simple remedy to the issue as it's described in the original report and posts #'s 1-3. My reply is aimed at the lower-level user/newbie who may come across this thread with the same issue and look at the suggested code fixes and patches as beyond their ability or overwhelming.

I had a user yesterday experience the same issue with a site that has the same install as the original poster (rickdonohoe). The issue was a result of the proper text format (Full) not being selected in the create/edit node page (right below the WYSIWYG editor). Once changed to "Full", all image properties held and were saved.

I didn't see this mentioned in any of the replies/comments and again, I'm only suggesting this as a first step for the lower-level users to look for or at least rule out before getting ahead of themselves. I'm also not suggesting that this wasn't done by any of the many helpful members who suggested their fixes in their replies; there's clearly a lot talent here. Hope it might help someone. Cheers!

I finally have the right combination of input filters and this patch to make this sort of work, but I'm getting errors on the submit button to actually add new images in the wysiwyg. When I get to the second screen of the add media dialogs (where I chose which display mode to use) I can't submit the form. Looking in the JS console, clicking submit gives me:

Uncaught TypeError: Object #

Added field overrides as an issue.

Re #61 @rickdonohoe and #63 @mariacha1: Are you using the latest 2.x-dev version of media or the alpha2 release? At this point, with a recent update to the 2.x-dev branch (see #2104839: extract_file_info does not call allowed_attributes() properly), all testing of patches on this thread should be done on the 2.x-dev branch. If you are getting a "false" or " " where your media should be, I'm relatively sure that's a result of trying this patch against the alpha2 branch. Let me know if that assumption is incorrect...

--------------------------------

Re #73 @ParisLiakos:

1) No, double-not-operator is intended. @see http://stackoverflow.com/questions/784929/what-is-the-not-not-operator-i...

2) Good point; went to update the Drupal.media.filter methods, but it looks like I overdid it and cleaned up the deprecated hooks. I think this is the cause of the issue in #76. Will revert that once I get a test or two written and a new patch rolled.

--------------------------------

It's great to see we're gaining some momentum on this thread. It's becoming obviously clear that this issue is crazy complex and has a lot of moving parts to it.

I'm attempting to standardize some of the testing behind this issue to alleviate some of the back and forth. A few things to note:
* Be sure you're using the latest 2.x-dev versions of file_entity and media
* Ensure your Drupal core is up-to-date with 7.23. At this time, we should not be testing against the 7.x-dev branch to avoid potential complications with unstable code.
* Wysiwyg version 2.2 w/ CKEDITOR 3.6.6.1 and/or TinyMCE 3.5.9. 4.x versions of these two editors are not fully stable with Wysiwyg. see #1968318: Support for TinyMCE 4.x and #1853550: Ckeditor 4.0 - The version of CKEditor could not be detected.
* Ctools 1.3

For any bug reports, please include your browser, version, and OS.

Thanks again for all the feedback and testing! Formal tests coming soon...

I've got #57 working correctly in a vanilla install with CKEditor.

Also marked #2102627: Updated media filter tags dont display correctly as a duplicate.

I guess we are still waiting on tests.

add issue

Re last comment comment #76, see #66 on #1504696: Integration with CKEditor module

@kaidjohnson i have some tests ready, will post them soon..i uploaded the issue summary with manual testing instruction

Add manual testing instructions

I agree with @kaidjohnson's comment, if there are existing problems using ckeditor v4 and TinyMCE v4 then lets keep those out of scope right now.

so after i wrote the tests, i fixed them too..the patch is quite different from #57, but it works for me on tinymce and ckeditor in chrome..no matter if we sorted the attributes, still the markup is different from the tagmap, since eg tinymce will add its own attribute data-mce...
tests include overwriting fields and attributes in wysiwyg
We need to add one more test, for other fields besides, alt and title because they have special handling

hmm lets add token as dependency..title and alt fields wont be displayed without the token module

well yes, bot wont checkout token module, so we need to actually make this a dependency before re-testing this patch.

Setup vanilla install as described in issue summary then applied https://drupal.org/files/media-2067063-84.patch.
Tested in Chrome.

Uploaded image with tags 'old', then set them to 'new' on the next screen.

<p>[[{"fid":"2","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"new","field_file_image_title_text[und][0][value]":"new"},"type":"media","tagName":"IMG","src":"http://d7.local.com/sites/default/files/higherlight.jpg","attributes":{"alt":"new","class":"media-element file-default","height":1800,"title":"new","width":2880}}]]</p>

<img alt="new" class="media-element file-default" height="1800" title="new" width="2880" typeof="foaf:Image" src="http://d7.local.com/sites/default/files/higherlight.jpg">

Click image and click media button to edit image. set alt tag to empty.

<p>[[{"fid":"2","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"","field_file_image_title_text[und][0][value]":""},"type":"media","attributes":{"class":"media-element file-default","height":1800,"width":2880},"tagName":"IMG","src":"http://d7.local.com/sites/default/files/higherlight.jpg"}]]</p>

<img class="media-element file-default" height="1800" width="2880" typeof="foaf:Image" src="http://d7.local.com/sites/default/files/higherlight.jpg" alt="">

Should this have returned the default tags instead of ""?
Double click image and set width and height to 80x50 in ckeditor

<p>[[{"fid":"2","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"new","field_file_image_title_text[und][0][value]":"new"},"type":"media","tagName":"IMG","src":"http://d7.local.com/sites/default/files/higherlight.jpg","attributes":{"alt":"new","class":"media-element file-default","height":1800,"style":"width: 80px; height: 50px;","title":"new","width":2880}}]]</p>

<p>
<img alt="new" class="media-element file-default" height="1800" style="width: 80px; height: 50px;" title="new" width="2880" typeof="foaf:Image" src="http://d7.local.com/sites/default/files/higherlight.jpg">
</p>

Set border, hspace and vspace to 5, and changed the alt text to 'ckeditor'. also set alignment to right:

<p>[[{"fid":"2","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"new","field_file_image_title_text[und][0][value]":"new"},"type":"media","tagName":"IMG","src":"http://d7.local.com/sites/default/files/higherlight.jpg","attributes":{"alt":"ckeditor","class":"media-element file-default","style":"width: 80px; height: 50px; border-width: 5px; border-style: solid; margin: 5px; float: right;","title":"new"}}]]</p>

<img alt="ckeditor" class="media-element file-default" style="width: 80px; height: 50px; border-width: 5px; border-style: solid; margin: 5px; float: right;" title="new" typeof="foaf:Image" src="http://d7.local.com/sites/default/files/higherlight.jpg">

Unset all the CKEditor settings:

<p>[[{"fid":"2","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"new","field_file_image_title_text[und][0][value]":"new"},"type":"media","tagName":"IMG","src":"http://d7.local.com/sites/default/files/higherlight.jpg","attributes":{"class":"media-element file-default","height":1800,"title":"new","width":2880}}]]</p>

<img class="media-element file-default" height="1800" title="new" width="2880" typeof="foaf:Image" src="http://d7.local.com/sites/default/files/higherlight.jpg" alt="">

If you set the alt tag in the ckeditor settings, the alt tag will be empty in the markup. But editing the image via media will set it again.

Anything else I should test while I'm at it?

Should token be a dependency of file entity instead, since that's where the alt/title tokens initially need to get defined?

Wrt the use of the token module please look at -- https://drupal.org/node/1553094#comment-6718168

Paris, thanks for the test! The javascript updates included in your patch (#84), however, undo a few crucial fixes implemented in #57.

Here are a few step-by-step reproducible issues that we should not lose track of:

1. If the user edits the macro directly, but then changes back to the rich-text markup, the macro will not render properly.
   To reproduce:
   • Insert an image into your editor.
   • Toggle to plain text using the 'Disable rich-text' link.
   • Type in a class of 'foo' into the class attribute within the macro.
   • Toggle to rich text using the 'Enable rich-text' link.
   • Result: Image is now broken in the editor. Expected result: Image remains in tact in the editor with an additional class of 'foo'.

   Cause: We are relying on a tagmap caching mechanism to re-render the markup from a macro, which under these circumstances no longer exists.
   Proposed Solution: Attempt to use a cached version, if it exists, but if it doesn't, re-build the element from the data in the macro. Note that this process does require storing the tagName and the src attribute in the macro.

2. If the user attempts to update the title or alt fields using the media popup, all attributes/changes are stripped from the media in the editor.
   To reproduce:
   • Insert an image into your editor.
   • Double-click on the image to show the Wysiwyg image editor.
   • Set the image to be right aligned, adding a 'float: right' on the image.
   • Press ok to apply this 'float: right' style.
   • Select the image again, but this time use the media popup button to pull up the media pop editor.
   • Change the title field to 'foo'.
   • Press save to apply this change.
   • Result: image is no longer floating right. Expected result: image is still floating right and has its title attribute updated to 'foo'.

   Cause: The insert method on the InsertMedia prototype object is explicitly setting the attributes to whatever was passed in via the media popup.
   Solution: The media popup only really affects the 'title' and 'alt' attributes, so extract those and merge those into the existing attributes object.

This patch is a combination of the tests put forth in #84, the solutions to the issues above from #57, some cleanup and extra documentation, and one bugfix that addresses the issue of storing the src urls in the macros and having the auto-link url feature on.

Also, I think it makes sense to put token as a dependency on file_entity if that is where the token handling originates.

re 2) This should be fixed on the php side and not js. The overriden fields are stored in the macro. Putting this logic in PHP means that any token containing field overrides will render correctly without needing to go through js. check #84 for a php solution

about 1) I have a feeling this is out of scope of this issue and an edge case.

i opened #2114793: Alt and title tokens do not render when token module is not installed for the token issue

Solution: The media popup only really affects the 'title' and 'alt' attributes, so extract those and merge those into the existing attributes object.

This effects more than alt and title - any field that has the "Override in WYSIWYG" checkbox selected will appear on the file's edit form in the WYSIWYG, but only usable for text and long text fields. See: #2062721: Add a white list of file fields that can be overwritten when the file is added in the wysiwyg

@ParisLiakos re #92 -- Part 1) I can't find the specific comment off hand, but this issue was mentioned in this thread at one point. Should the macro change in any way via the wysiwyg (not just manual editing, but also field updating, and the original hspace/vpsace/float issue this thread stemmed from) and the user toggles the rich-text editor on and off, the images will disappear if we're relying strictly on a tagmap to call back the rendering. There has to be a javascript solution in place in the event that the macro is not found in the tagmap to rebuild the markup from the macro itself. I believe this particular portion of the issue is the other significant half of this issue as a whole; the first half being the php handling of the macro itself.

@ParisLiakos re #92 -- Part 2) I agree that there must also be a php solution in place, however the issue also must also be addressed in the javascript. Take the following example:

• Add a new image into the wysiwyg, give it a title field value of 'Hello'
• Select the image, update the title field via the media popup to 'Goodbye'
• Double-click on the image, opening the wysiwyg image properties window, head over to the 'Advanced' tab and enter an Advisory Title of 'Whatever'

The same issue applies to the 'alt' tag handling. See comment #53 for another use-case.
A purely php solution would override the 'Whatever' with 'Goodbye'. We have to persist changes between the WYSIWYG Image Properties and the Media Popup field editor to ensure that users can manage those settings between them. It may be that this particular portion should have its own issue thread once we resolve the core issue at hand.

@bneil re #93 -- I meant that the alt and title fields are the only two entity fields that are also HTML attributes, and thus need to be updated when the file entity fields are updated; other fields, although updatable via the media popup, don't affect the markup. EDIT: It's not just that they are HTML attributes, but the alt and title fields can be edited/updated via the WYSIWYG Image Properties window. Other fields are safe from that alternative editing area.

Testing with media_dev distro, updated file_entity, media, token and applied patch #90. Installed TInyMCE 3.5.9.

With FF on Mac, inserting an image saves as img instead of media macro. In replacePlaceholderWithToken, the

 content = content.replace(markup, macro);

fails because the img in markup is not exactly the same as img in content.

I thought maybe a regular expression would work better. Something like this.

        var rgx = /<img[^>]*media-element*[^>]*>/;
        content = content.replace(rgx,macro);

After I made this change, the image is saved as media macro. But, I found one problem when resizing an image. Say the original is 960 by 720. Entering 300 for width and leaving height empty should resize the image and keep aspect ratio. It looks OK in the editor. But after saving, the new width is used (300) and the original height (720).

@ellen.davis -- Thanks for the update. The regex might not be the best solution as we need to ensure this code works with media elements aside from just images. I'll have to do some testing, though. I'll also look into why FF doesn't want to match up properly. This may be a regression caused by the alphabetizing of the element attributes or something else entirely.

Just to be clear. Testing on FireFox 24 on Mac.

CKEditor does match up and the media macro is saved.

TinyMCE does not match up so the img tag is saved.

Another idea I had was to make TInyMCE alphabetize the img elements as CKEditor does. This seems to accomplish that.

function tinymce_img_wysiwyg_editor_settings_alter(&$settings, $context) {
  if($context['profile']->editor == 'tinymce') {
  $settings['extended_valid_elements'] .= ',img[alt|class|data-file_info|fid|format|height|src|style|title|view_mode|width]';
  }
}

With the above, the

content = content.replace(match, markup);

matches in replaceTokenWithPlaceholder for TinyMCE and the media token is saved.

But I still have the same issue with the image re-sizing, as described in #95.

media.filter.js assumes that anything inside [[ ]] tags is a media tag. But node embed uses the following: [[nid:671]].

This breaks node embed as media filter tries to convert the tag, leading to

media.filter.js:41 : Uncaught TypeError: Cannot read property 'tagName' of undefined

and a submit button that won't submit on the node embed form.

Some checking that the tag is actually a media tag is probably required.

    replaceTokenWithPlaceholder: function(content) {
      var matches = content.match(/\[\[.*?\]\]/g);

      if (!!matches) {
        for (i in matches) {
          var match = matches[i];

          // Check if the macro exists in the tagmap. This ensures backwards
          // compatibility with existing media and is moderately more efficient
          // than re-building the element.
          var media = Drupal.settings.tagmap[match];
          var media_json = match.replace('[[', '').replace(']]', '');

          // Ensure that the media JSON is valid.
          try {
            var media_definition = JSON.parse(media_json);
          }
          catch (err) {
            // @todo: error logging.
          }

          // Only re-build the media if the macro has changed since the tagmap
          // was last built.
          if (!media) {
            media = document.createElement(media_definition.tagName);
            media.src = unescape(media_definition.src);
          }

For people having issues with the patch in #57, try to put the media filter BEFORE any other filter in your text formats configuration...

As always, thanks for the feedback.

@samhassell re #99: Good catch. This patch now should only match tags that have "type":"media" in them, so that issue should be taken care of.

@ellen.davis re #95 - #98: The issue you point out here was reproducible in more than just OSX/FireFox; it was an issue that was caused by a subtle difference in the way TinyMCE handles attribute ordering (or doesn't, I should say). I think I found a much more x-browser way to handle the matching and replacing of the markup with macros. Instead of trying to fudge a solution between jQuery matching and the javascript replace() method, this patch uses strict NodeList replacing. I've tested this out on Linux/Chrome 30/CKEditor 3.6/TinyMCE 3.5.9, Linux/FireFox 22/CKEditor 3.6/TinyMCE 3.5.9, and Windows 7/IE9/CKEditor 3.6/TinyMCE 3.5.9 and it worked solidly for singular and multiple images. This patch also drops the whole safeMarkup nonsense that was my previous far-too-hacky attempt at x-browser stability.

@mariacha1 re #68: I was able to reproduce the issue where if you set the view mode to anything other than default, height and width settings from the WYSIWYG don't get applied. Digging in a bit, it seems that the cause is image_style_transform_dimensions() as this is where the core image rendering applies the image style settings. Unfortunately for us, we're invoking drupal_render() on theme_image_formatter(), which invokes theme_image_style(), which invokes image_style_transform_dimensions(), which overwrites the passing along of custom dimensions. If anyone has any ideas of how to address this problem.... We should avoid a hacky preg_replace of attributes post-rendering. This particular point, though, is out of scope of this thread and should be moved to its own issue queue for follow-up.

Patch in #101 works well for me (note: tested in CKEditor 4 - I know you're not interested in results for that version but that's all I've got available right now).

re-roll of the patch by @kaidjohnson in #101 which applies to the latest dev.
Only differences:
removes test_dependencies[] = token as that is added as a normal dependency with this patch.
some line numbers and of course the index hash.

Will test the patch now, although also on WYSIWYG-dev+CKeditor 4.x.

Forgot that.
And forgot to send kaidjohnson kudos for his awesome work here.

Has anyone tested these patches with lets say the youtube sub-module?
I've found that after I think patch 42 if you're trying to insert a video you get no output. As in once you select and submit no code is insert into wysiwyg.
Anyone else experience this? Or am I missing something? The latter being more likely...

EDIT: Also, kudos to kaidjohnson for taking the lead and their hard work!

The test is failing with the following error:

PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'file_managed.type' in 'where clause': SELECT file_managed.fid AS entity_id, :entity_type AS entity_type, NULL AS revision_id, :bundle AS bundle FROM {file_managed} file_managed WHERE (file_managed.type = :db_condition_placeholder_0) ; Array ( [:db_condition_placeholder_0] => image [:entity_type] => file [:bundle] => file ) in EntityFieldQuery->execute() (line 1140 of /var/lib/drupaltestbot/sites/default/files/checkout/includes/entity.inc).

@jags880 i noticed the same behaviour with media_vimeo. See comment #67

@jags880 and @mallezie -- Both vimeo and youtube appear to be working for me. The only scenario I ran into was with TinyMCE when the video file display settings (admin/structure/file-types/manage/video/file-display) had a default setting of 'Video' enabled. If you disabled that setting do those plugins now work for you? If not, could you provide more information about wysiwyg library/version/browser/os? The 'Vimeo Preview Image', 'Vimeo Video', 'YouTube Preview Image', 'YouTube Video' all appear to be working as expected; the preview images mimicking other image handling and the vimeo / youtube video setting embeds a link placeholder that renders into the full-sized video on the front-end. It's not 100% ideal because you wouldn't be able to alter the size of the videos themselves, but that's an issue for another thread and may be something to be addressed by each of those sub-modules. I think videos should have set placeholders for the WYSIWYG representation, regardless of view mode, and then render the selected view mode according to the file display settings for the front end presentation only; but that's definitely out of scope of this thread.

@aaron -- we were having a similar problem before. see #2107063: SimpleTests are failing. and #2109293: Media's 7.x-2.x dependencies are not being downloaded. I'm wondering if that is once again plaguing us or if this is more like what Paris is experiencing in #2114793: Alt and title tokens do not render when token module is not installed. fwiw, all tests are passing cleanly locally (see attached screenshot).

In my patch from #101, I forgot to include the new tests from ParisLiakos in #84 and the reroll in #104 thus was missing it. Re-rolled again to include the tests/media.macro.test file.

ahem... whoops.

Argh. Rolled the patch in the wrong way to handle the new file. Need to at least get past the 'detect a non-applicable patch'. Re-rolled. Take 2.

Patch won't apply against -dev now.

Here's a reroll. The only difference is

var index = matches.indexOf(macro);

is replaced with

var index = $.inArray(macro, matches);

in media.filter.js.

1. Add an existing image to the wysiwyg using the default image style.

<p>[[{"fid":"44150","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"sdfsd fsdf","field_file_image_title_text[und][0][value]":"f sfsdfs","field_copyright_owner[und][0][value]":"","field_copyright_year[und][0][value][year]":"","field_caption[und][0][value]":"","field_caption[und][0][format]":"summary_html"},"tagName":"IMG","src":"http%3A//gateway.local.com/sites/gateway/files/styles/landscape_large/public/02215_docadocavacas_2560x1440.jpg%3Fitok%3Dn5LTyzSr%26c%3Dc651ef91899321ab16d3203cd7af02ac","type":"media","attributes":{"alt":"sdfsd fsdf","class":"media-element file-default","height":480,"title":"f sfsdfs","width":639}}]]​</p>

2. Click the image and click the media gallery item to edit the image
3. Change the image style and submit the form.
(Image size changes)
4. Click the image
(Media gallery icon doesn't fade)
5. Click media gallery image
(Goes to add new image instead of edit image)

Checking the tag shows as follows:

<p>[[{"fid":"44150","view_mode":"landscape_medium","fields":{"format":"landscape_medium","field_file_image_alt_text[und][0][value]":"sdfsd fsdf","field_file_image_title_text[und][0][value]":"f sfsdfs","field_copyright_owner[und][0][value]":"","field_copyright_year[und][0][value][year]":"","field_caption[und][0][value]":"","field_caption[und][0][format]":"summary_html"},"tagName":"A","src":"undefined","type":"media","attributes":{"alt":"sdfsd fsdf","class":"media-element file-default file-landscape-medium","height":"480","title":"f sfsdfs","width":"639"}}]]​</p>

"src":"undefined" seems strange..

*edit* "tagName = A" may be the reason here. the file display uses colorbox for output.

Is there a reason why the image source "IMG","src":"http%3A//gateway.local.com/sites/gateway/files/styles/landscape_large/public/02215_docadocav" needs to be stored in the database? We really don't like internal URLs or development server information stored in a database that may eventually be moved to a production system.

Also, as far as I can tell, this information is not needed in the database, since the value is reset when the node edit form is shown. (I tested this by purposefully breaking the url in the database and then reloading the node edit page, the value was reset correctly...) From what I can tell this is only used so the editor can display the correct preview image in the wysiwyg.

FYI it may be worth testing out #1956778: Ckeditor 4.1 ACF as well.

Tested out #115 in an install that also included CKEditor 4.2.2 and WYSIWYG 7.x.2.x-dev with patch 37 from here applied https://drupal.org/node/1956778 and I got a successful result of being able to align images added inline via Media to the left and right.

Retested #116 using a proper vanilla install of drupal 7.23, wysiwyg-7.x-2.x-dev, file_entity-7.x-2.x-dev, media-7.x-2.x-dev & ckeditor 4.2.2
Applied both media-broken-wysiwyg-2067063-115.patch and wysiwyg-ckeditor-acf-1956778-37.patch.

Set up ckeditor, wysiwyg, and text filter.

Insert image into article using the default file display.
Image appears.

<p>[[{"fid":"3","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"wesdfsdf","field_file_image_title_text[und][0][value]":"sdfsdf"},"tagName":"IMG","src":"http%3A//d7.local.com/sites/default/files/0C2Y8.jpg","type":"media","attributes":{"alt":"wesdfsdf","class":"media-element file-default","title":"sdfsdf"}}]]</p>

Edit image using WYSIWYG button and choose 'teaser' for file display.
Image appears smaller.
Clicking image doesn't dim media wysiwyg icon.

<p>[[{"fid":"3","view_mode":"teaser","fields":{"format":"teaser","field_file_image_alt_text[und][0][value]":"wesdfsdf","field_file_image_title_text[und][0][value]":"sdfsdf"},"tagName":"A","src":"undefined","type":"media","attributes":{"alt":"wesdfsdf","class":"media-element file-default file-teaser","height":"1600","title":"sdfsdf","width":"2560"}}]]</p>

No ckeditor involved and it still gets tagName as A hrm..

*edit* fixed typo in ckeditor version

Given CKEditor is up to 4.2.2, I don't think there's any point in testing any v4 release that's older than 4.2.0, we can always just state in the docs that we have minimum requirements for CKEditor.

Rerolled against v7.x-2.0-alpha3+3-dev.

Doh.

@samhassell re #116 / #120 -- Thanks for the use case. I can confirm that the tagName: 'a' is the issue. The image formatter kicks in before the wysiwyg inserts markup and the colorbox setting will wrap your images in an <a> tag. Incidentally, if you save it successfully, it gets wrapped again when the image is rendered on the front-end, causing duplicate <a> tags. The proposed solution would be to strip out a.colorbox when the media markup is processed in the WYSIWYG. The colorbox will render fine on the front-end and prevents tag duplication. This patch update should resolve the issue.

@joelcollinsdc re #117 -- You bring up some very valid points. I have moved storage of the tagName and src attributes to be used only during WYSIWYG interactions, thus preventing database storage of these attributes. This update should take care of that issue.

@DamienMcKenna -- any updates in your re-roll or just chasing HEAD? This patch was just rolled, so should be up-to-date.

Many thanks to everyone for the continued testing and feedback.

I'm setting the issue to needs review, but expect the patch to fail for the same reason mentioned in comment #5 of #2114793: Alt and title tokens do not render when token module is not installed. Screenshot attached of successful local tests.

@DamienMcKenna, the ckeditor version in #120 was a typo - im using 4.2.2.

@kaidjohnson, the anchor tag seems to be getting the img tags properties now. Note that this is without colorbox. By default the teaser file display gets wrapped in a link to file/%fid. The other displays (Preview / Default) work fine.

This code is pulled out of the inspector when viewing the image in the wysiwyg:

<a alt="alt" class="media-element file-default file-preview file-teaser" data-file_info="%7B%22fid%22:%224%22,%22view_mode%22:%22teaser%22,%22fields%22:%7B%22format%22:%22teaser%22,%22field_file_image_alt_text%5Bund%5D%5B0%5D%5Bvalue%5D%22:%22alt%22,%22field_file_image_title_text%5Bund%5D%5B0%5D%5Bvalue%5D%22:%22title%22%7D,%22type%22:%22media%22%7D" height="1080" data-cke-saved-href="/file/4" href="/file/4" title="title" width="1920"><img alt="alt" data-cke-saved-src="http://d7.local.com/sites/default/files/styles/medium/public/03IgESs.jpg?itok=AI7ecPYq" src="http://d7.local.com/sites/default/files/styles/medium/public/03IgESs.jpg?itok=AI7ecPYq" title="title" typeof="foaf:Image"></a>

@kaidjohnson: Yeah, sorry, not sure why my local Media install had a problem with the previous patch.

Test environment:

• Drupal v7.22
• WYSIWYG v7.x-2.2+23-dev with the patch from #1956778: Ckeditor 4.1 ACF.
• File Entity v7.x-2.0-alpha3+1-dev
• Media v7.x-2.0-alpha3+3-dev with the patch from #124.
• CSS aggregation disabled, including disabling AdvAgg v7.x-2.1.
• Content type with two WYSIWYG fields.

Test scenarios:

• Add one image to one field: worked!
• Add one image to both fields: worked!
• Replace one of the two images: worked!
• Add several additional images to one field: worked!
• Highlight one of the images, click the Media Browser button and edit the file's details: worked!
• Change alignment to one image: worked!
• Make one image a link: worked!
• Disable & re-enable WYSIWYG editor: worked!
• Edit source code via CKEditor's toolbar: worked!
• Change image display mode, alt & title text during image uploading: worked!
• Change image display mode, alt & title text after image uploading: worked!

Definition: "worked!" means that the image saved correctly and all values were still present once the edit form was loaded again.

i don't want to seem editor-ist ;) but I'm sticking with CKEditor and the above combination.

When changing the file display, the html width and height attributes are not getting updated any more.

It looks like they are getting set automatically in the Image Properties dialog, but then not overridden after altering the image in media.

This issue is damn fiddly :)

@sahmhassell: I re-tested it after your comment and the meta values were all saving correctly for me.

hrm ive got the same setup as you except im on 7.23 not 7.22.

I keep needing to remove the width and height from image properties after changing the file display..

see: https://www.dropbox.com/s/1w5jhwgc9ia65hw/out.ogv

I can fix my resizing issue with the following patch on top of #124. Also the anchor tag issue outlined in #125 can be fixed by changing the colorbox specific code to just target wrapping anchor tags. Am I missing anything obvious here?

diff --git a/js/media.filter.js b/js/media.filter.js
index 311ccf8..3c60525 100644
--- a/js/media.filter.js
+++ b/js/media.filter.js
@@ -133,9 +133,9 @@
       }
       var element = $(html);
 
-      // Parse out colorbox wrappers. They will be re-applied when the image is
+      // Parse out link wrappers. They will be re-applied when the image is
       // rendered on the front-end.
-      if (element.is('a.colorbox')) {
+      if (element.is('a')) {
         element = element.children();
       }
 
@@ -277,7 +277,7 @@
      * In case of an error the default settings are returned.
      */
     allowed_attributes: function () {
-      Drupal.settings.wysiwyg_allowed_attributes = Drupal.settings.wysiwyg_allowed_attributes || ['height', 'width', 'hspace', 'vspace', 'border', 'align', 'style', 'alt', 'title', 'class', 'id', 'usemap'];
+      Drupal.settings.wysiwyg_allowed_attributes = Drupal.settings.wysiwyg_allowed_attributes || ['hspace', 'vspace', 'border', 'align', 'style', 'alt', 'title', 'class', 'id', 'usemap'];
       return Drupal.settings.wysiwyg_allowed_attributes.sort();
     }
   }

patch with the above suggested changes included.

This is quite the house of cards, but the setup in #127 plus the patch from #132 are working for me. I'm passing this along to our QA engineer and I'll let you know if anything comes up.

Patch #132 works great in CKEditor for me.

But I have an issue with TinyMCE. The height/width attributes are not saved. I can resize an image in the editor by using the handles. It looks fine in the editor. But after saving, view shows the original sized image. Edit again shows the original sized image in the editor.

TinyMCE sets the width/height using the width and height attributes. Not the style.
http://www.tinymce.com/develop/bugtracker_view.php?id=5918

IF I add width and height back into the wysiwyg_allowed_attributes, then TinyMCE will save width and height.

Drupal.settings.wysiwyg_allowed_attributes = Drupal.settings.wysiwyg_allowed_attributes || ['height','width','hspace', 'vspace', 'border', 'align', 'style', 'alt', 'title', 'class', 'id', 'usemap'];

However, this seems to conflict with the issue that samhassell mentions.

@Damien - It really does work!!!!

The only other change I did was using, Media-7.x-2.x-dev with the patch from #132 over #124.
We have an editorial site that's gonna go live very soon. I was working on alpha-2, and this is a functionality that is a core requirement of their site, so I couldn't be more pleased at the lucky timing.

I just want to sneak in one small question. On the server, is drush updb a must, after upgrading media(7.x-2.0-alpha2) and wysiwyg module, right? Or will I have to uninstall the previous modules (media & wysiwyg), and replace it with the the patched dev ones. On my local, I didn't have to do anything, just replacing the code worked, but I'm sure that's not the right practice.

Cheers.....Had assigned myself the entire weekend to fix this. Couldn't be better timing :-)

@samhassel re #132 -- Yeah, that makes sense to check for any pre-wrapped <a> tags -- any image that is pre-wrapped like that will get re-wrapped when rendered on the front end, and any changes the user might want to make to that wrapper won't persist anyway due to the re-wrapping. Good call.

@samhassel re #132, @ellen.davis re #134 -- I observed this issue in comment #101 in response to #68. It appears that the issue affects TinyMCE more directly; without the 'height' and 'width' being whitelisted, TinyMCE doesn't handle those attributes correctly:

(from #101) -- Digging in a bit, it seems that the cause is image_style_transform_dimensions() as this is where the core image rendering applies the image style settings. Unfortunately for us, we're invoking drupal_render() on theme_image_formatter(), which invokes theme_image_style(), which invokes image_style_transform_dimensions(), which overwrites the passing along of custom dimensions. If anyone has any ideas of how to address this problem.... We should avoid a hacky preg_replace of attributes post-rendering. This particular point, though, is out of scope of this thread and should be moved to its own issue queue for follow-up.

refs:
image_style_transform_dimensions()
theme_image_formatter()
theme_image_style()

I still believe that, for now, we should leave the 'height' and 'width' in place for TinyMCE support. If I recall, the issue is only present when the user selects an image derivative, which of course is quite frequently, so the issue does need to be addressed. Because the issue is deep within the rendering thread and not a direct cause of anything specifically with the WYSIWYG integration, I vote for moving the issue to a new thread when this one gets wrapped up. Thoughts?

Here's the update from #132 without the 'height' and 'width' removed from the allowed_attributes.

i agree that width and height should stay to allowed_attributes for now..at least in this issue

+++ b/media.info
@@ -6,8 +6,7 @@ core = 7.x
-test_dependencies[] = token
+dependencies[] = token

this change should be reverted. If token becomes a dependency, its up to file_entity to add it. issue is already created at #2114793: Alt and title tokens do not render when token module is not installed

also, another related issue #2120997: Revert media wrappers in WYSIWYG editors

No sooner did I post #136 and a thought occurred to me -- the reason that CKEditor will render the image height/width properly is because its override mechanism inherently creates a style attribute and sets the height and width there. This attribute is not overridden during rendering, and carries more weight in the browser so the image appears resized. TinyMCE only alters the height and width attributes directly. Interestingly, if you manually add a style attribute with height and width then resize the image again, TinyMCE WILL update those values... a little odd, but we can take advantage of it.

This patch includes the recommendation in #137 and ensures that a style attribute with height and width are set on the image (and are synced with the height and width attributes for consistency). This should address the issue from #130/#134/#136. The x-browser implications in this little bit are hairy, at best, but it does the job.

Neither the 'data-picture-align' nor the 'data-picture-group' attributes are being written to the media tag.

Set-up Info

• Drupal 7.23
• Media 7.x-2.0-alpha3+3-dev
• Patched with #138
• WYSIWYG 7.x-2.2
• CKEditor 4.2
• Picture 7.x-1.2

Steps to Reproduce

• Enable 'Picture' module
• Enable 'Make images responsive with the picture module' filter in text formats and order after 'Convert Media tags to markup' filter
• Enable the 'Responsive images with the Picture Module' plugin in the WYSIWYG profile for the same text format
• Insert an image into WYSYWYG editor
• Edit 'Image Properties' for the image
• -Set 'Image Alignment' to 'Left'
• -Set 'Image Size' to any group
• -Click 'OK' to save changes
• Disable rich text to view media tag

Below is the result these steps produce for me:

[[{"fid":"2","view_mode":"default","fields":{"format":"default","field_file_image_alt_text[und][0][value]":"alt here","field_file_image_title_text[und][0][value]":"title here"},"type":"media","attributes":{"alt":"alt here","class":"media-element file-default","height":1728,"style":"height: 1728px; width: 2304px;","title":"title here","width":2304}}]]

You will observe both the 'data-picture-align' not the 'data-picture-group' attributes are omitted; however, both are included in the '_media_wysiwyg_allowed_attributes_default' function in patch #138.

Is this related to this Media issue or should this be filed under the Picture module?

Added more related issues.

Update to #139

Scenario 1

Save with rich text disabled

1. Disable the rich-text toggle
2. Manually add a 'data-picture-group' attribute and value
3. Save without enabling the rich-text toggle

Result: The media tag is rendered correct on page display.

Scenario 2

Toggle rich text

1. Disable the rich-text toggle
2. Manually add a 'data-picture-group' attribute and value
3. Enable the rich-text toggle
4. Disable the rich-text toggle

Result: The 'data-picture-group' attribute is no longer present.

Scenario 3

Remove 'Picture' module from test and toggle rich text

1. Disable 'Make images responsive with the picture module' filter in text formats
2. Disable the 'Responsive images with the Picture Module' plugin in the WYSIWYG profile for the same text format
3. Disable the rich-text toggle
4. Manually add a 'data-picture-group' attribute and value
5. Enable the rich-text toggle
6. Disable the rich-text toggle

Result: The 'data-picture-group' attribute is no longer present.

The result of Scenario 3 suggests an issue with 'Media' and not with 'Picture'.

Our bot problems just got fixed, so i went ahead and committed a close version of patch in #84 here as a stopgag/temporary solution. That commit must have also fix a bunch of the related issues.

We can now narrow down the scope of this issue a bit more and tackle problems more focusedly

@ParisLiakos -- Thanks for getting a solid foundation of this issue committed to the project!! A great first step that alleviates a majority of the critical pieces to this issue.

@Chris Burge -- It appears that we weren't properly passing the default allowed_attributes from php to javascript, so the javascript list wasn't in sync with the base media settings. I've corrected the issue in this patch and your issue should be resolved.

This patch is a re-roll of #138 to account for the commit in #141.

I should note that I've dropped the allowed_attributes() method in media.filter.js because I believe it was originally created to get around the issue of the wysiwyg_allowed_attributes not being properly passed from the php to the javascript. I can't imagine a scenario where the value would be undefined now that the correct object is being used. Instead of attempting to manage a set of defaults in both the js and the php, I'm opting to rely on whatever is set in the php and using that in the javascript. This update addresses the issue outlined in #139/#140, caused by the javascript whitelist not matching the php whitelist.

I've also addressed another scenario that I ran into earlier this evening: When colorbox is used as the default file display for the image content type, the $element['theme'] (line 680, media.filter.inc) is set to 'colorbox_image_formatter', which appears to take the same arguments as 'image_formatter'. Of course, this theme doesn't match our switch() statement and the necessary attribute handling isn't done. There is an issue with colorbox_image_formatter, however, in that it doesn't honor all attributes. This will inherently interfere with our attempts to get attributes such as 'data-picture-group' to pass properly when using colorbox as the image style. See #2086403: Adopt image attributes in theme_colorbox_image_formatter. The patch in that thread seems to do the job.

There was a bug in the new tests. I was getting local failures on the testing of 'height="100" width="100"' for default attributes. My browser was rendering them as 'width="100" height="100"'. I don't think we can rely on the order being consistent so I blew that singular test out into two 'width="100"' and 'height="100"' tests.

Many many thanks to everyone!

@ParisLiakos re #141: Hurrah! Well done for getting something committed and narrowing the scope of this issue - lord knows it needed it. Onwards!

After some careful consideration and knowing that a foundational patch has been committed, it's time we break out a few of the remaining points of this issue so they can be addressed individually. I believe this will help us address these issues in a more efficient way to get them committed to the project as quickly as possible.

In the meantime, if you're in a pinch, patch #144 cumulatively addresses those issues and has been quite stable in my individual testing.

So far I have created 3 new issues with proposed solutions to address a few of these minor issues:

• re #144, false negatives in tests: #2126665: False negatives on test 'Image displays with default attributes.'
• re #144, alternative image formatters: #2126653: Wysiwyg -- theme_image_formatter overrides do not retain element attributes.
• re #139/#140: #2126661: Wysiwyg -- allowed_attributes() in media.filter.js does not properly sync with settings from the php

I have identified 3 remaining issues that the patch in #144 addresses, and will create those issues separately as well later this evening. Thanks for all the continued testing and support!

The final three separate issues have been created:

• #2126681: Wysiwyg -- registerNewElement method is never called and should be removed.
• re #68, #101, #132, and #134 -- #2126691: Wysiwyg -- Ensure height and width are always set in style attribute to avoid being overwritten during rendering.
• re #53 -- #2126697: Wysiwyg -- Alt and Title fields require some special handling.

What's left from the patch in #144 is the core macro handling updates that happened since the patch from #84. It consists of a major x-browser updates and a significant rewrite for macro creating and managing, and also addresses all of the bugs associated directly with that piece of the issue (such as image link wrappers, not storing src paths in the db, etc). Because that is the core issue of this Wyisywg thread and the code fully depends on itself to address certain issues (such as editing macros directly breaking the element rendering), I am leaving this work on this thread to be reviewed.

Note that I removed the two related threads that I added in #146 because those threads have been marked as children of this thread and are related to this thread as such.

I moved the last patch over to #2126755: Improve Media's WYSIWYG Macro handling
Lets move on to all the child issues created and listed on the right, since we are already on 150 comments here

I want to thank all of you and especially kaidjohnson for the work and testing on this issue!

Maybe someone can help me.. I'm experiencing this bug: #2028253: If you change a file's information with the WYSIWYG disabled, re-enabling the WYSIWYG loses the file. That issue claims it's a duplicate of this, but I have the latest dev and the issue is still very much there. I checked out the issue linked to in #148, and it doesn't seem to pertain to the bug I'm experiencing.

Has that one been addressed yet?

@rickdonohoe : Check the Child issues in the sidebar and if no-one covers one of your issues, please open a new one. thanks.
this issue is closed because the main and biggest bugs are now resolved, so we splitted the remaining in different ones

@rickdonohoe - many of the issues addressed in patch #147 have been resolved and committed to the project. Patch #147 is no longer relevant and will not be able to be applied to the latest dev snapshot.

I believe the last outstanding issue from that patch is #2126755: Improve Media's WYSIWYG Macro handling, which has been re-rolled to meet the change in module structure.

At this point, as pointed out by @ParisLiakos in #153, the issue this thread was addressing has been resolved; any remaining tasks associated with it have been created as child issues and can be followed as their own threads (see the Child issues of this issue in the upper-right sidebar here: https://drupal.org/node/2067063#block-views-e84165300884f34464eebe80f501...).

The image properties issue, which turned out to be a lot of different relatively-related, very-complicated issues, is fundamentally resolved and should be relatively stable for basic image embedding. If you continue to experience issues, check in with the child issues of this task or file a new task with the specifics of the issue you're running in to.

Thanks!

Please note, that with latest dev snapshots you'll still need one more patch from #2126697: Wysiwyg -- Alt and Title fields require some special handling. to get the alt/title text working properly on images embedded into a wysiwyg editor using the media embed filter.

I got alt/title text on images working with a combination of:

media-7.x-2.x-dev (alpha3+39-dev as of 2014-01-20) + patch in #2126697-6: Wysiwyg -- Alt and Title fields require some special handling.
file_entity-7.x-2.x-dev (alpha3+10-dev as of 2014-01-16)
wysiwyg-7.x-2.x-dev (snapshot packaged on 2014-01-22)
ctools-7.x-1.3

+1 for #156

I only needed the attributes in WYSIWYG editor to be respected and the bits jwilson3 recommended have worked for me.

Cheers

Tom

I can't have my attributes in wysiwyg and I came across this issue.

I was looking at the patches above and as far as I can see, most of this code is already in the latest dev version of Media. Anything I set in image properties is cleared after save.

Given the fact, that the latest comment is 4 months old and that Media had couple of updates of dev branch, does anyone else has problems with attributes?

Iam having the same issue like wheneever iam giving image float left or right option its not working when i save node.I can see many patches here for that.can anyone tell me which patch i need to apply for this issue
we are using WYSIWYG editor and TinyMCE.

Please reply someone

I am unable to link an image to a URL, as well as give them properties.

This issue is finished and has been closed.

Please create new issues for any problems you find (provided there is not already one that matches your issue).