To reproduce:
- Load up a page in the WYSIWYG with a media file embedded in it.
- Turn off the WYSIWYG and edit something about the file in the raw code (i.e., edit the text inside the
[[...]]
). Most realistically, you might imagine editing the link text if the file is being displayed as an embedded link. - Turn the WYSIWYG back on and your file is gone - you just see the embedded code. If you save afterwards, you lose the file.
Related previous issue (but not exactly the same): #834728: If you disable rich text and then re-enable, you can't get your wysiwyg formatting back until you publish
This is basically a consequence of how the Media module keeps track of the mapping between macros (the [[....]]
) and actual HTML. It uses the entire macro string as an array key, so if the string changes it no longer has any HTML to match it to.
I suppose to fix this, it would be necessary to use something else as the key (I was thinking a unique ID, perhaps generated in the JS layer with Math.random(), stored inside the macro). This would work, but would require some other work too as it would need to be able to generate the (new) HTML also.
Other ideas?
Comment | File | Size | Author |
---|---|---|---|
#10 | interdiff.txt | 646 bytes | azinck |
#10 | media-repair_encoded_macro-2028253-10.patch | 889 bytes | azinck |
#9 | media-repair_encoded_macro-2028253-9.patch | 892 bytes | azinck |
Comments
Comment #1
David_Rothstein CreditAttribution: David_Rothstein commentedFixing title.
Comment #2
azinck CreditAttribution: azinck commentedI think we should continue to use the full token as the key (since changes in the token, such as field overrides) can affect the rendering of the token. But we need to add a menu callback to which we can pass the full token to get a rendered version of it.
Comment #3
TwoDNo offence, but the current placeholder solution is horrible. (See also #2085997: WYSIWYG Image update not working.)
A multi-element placeholder is essentially impossible to control in WYSIWYG mode.
I'm currently experimenting with this since it's driving users on two of my sites nuts. If it isn't fixed soon, I'll have to ditch Media completely while I still have the option of re-building all the content using it.
So far:
The source order of placeholders/macros no longer matters because of this.
This still has massive problems with:
Next, I plan to basically implement what azinck suggested and pass the macro to the server when there's no match in the tag map and get a new placeholder from there. What worries me is the potential delay when doing this.
That still won't help a bit with the major issue of users not being able to change the link text "live". Not that I've yet to actually figure out how to find the closing a tag, or limit selection/manipulation to the topmost placeholder element while in WYSIWYG mode.
Comment #4
steven.wichers CreditAttribution: steven.wichers commented+1 that this is a big issue. Even minor actions like using alignment buttons in CKEditor are problematic because Media does not respect changes in its element's attributes.
Comment #5
kaidjohnson CreditAttribution: kaidjohnson commentedWe're addressing this issue in this thread: https://drupal.org/node/2067063 as it is related to the issues around attribute persistence.
EDIT: @TwoD, I completely agree with you about the wrappers. See thought (3) in https://drupal.org/node/2067063#comment-7936175.
Comment #6
mstef CreditAttribution: mstef commentedI'm confused..
I have the same problem. I followed the issue mentioned in #5. It claims it is fixed. I have the latest dev - it's not fixed. I followed the issue that was created out of the issue from #5. That patch does not pertain to this bug..
So.. maybe we need to reopen this?
Comment #7
ParisLiakos CreditAttribution: ParisLiakos commentedthe work on this splitted off to #2126755: Improve Media's WYSIWYG Macro handling
Comment #8
azinck CreditAttribution: azinck commentedI know #7 said this was to get taken care of in #2126755: Improve Media's WYSIWYG Macro handling but it never did. This is still a problem.
Comment #9
azinck CreditAttribution: azinck commentedI don't expect this patch will get accepted. But it will at least make the tagmap generation more tolerant when it hits an html-encoded macro. Without this your WYSIWYG editor won't load due to a javascript error:
TypeError: source is undefined
Comment #10
azinck CreditAttribution: azinck commentedOops, slight change. The fid might not be the first attribute listed in the macro.
Comment #11
Devin Carlson CreditAttribution: Devin Carlson commentedComment #12
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commentedClosing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team