Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In first lines of media/modules/media_wysiwyg/includes/media_wysiwyg.filter.inc file there are regex for searching media tokens:
define('MEDIA_WYSIWYG_TOKEN_REGEX', '/\[\[.*?\]\]/s');
This regex finds all [[anything]] blocks. But some other modules also an use this type of tokens, so using media will break those tokens from other modules.
Best solution is adding media prefix for this tokens, for prevent all other conflicts, so change this token:
[[{"fid":"24","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":"745","width":"1059","class":"media-element file-default"}}]]
to this:
[[media-wysiwyg:{"fid":"24","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":"745","width":"1059","class":"media-element file-default"}}]]
But this must be changed on all other modules, that use this token, too.
For easily fix this problem, we can extend regexp to find only file tokens, starting with {fid text - like this:
define('MEDIA_WYSIWYG_TOKEN_REGEX', '/\[\[\{"fid".*?\]\]/s');
I attach patch with this fixes.
---
P.S. Also in function _media_wysiwyg_generate_tagMap($text) there are duplicate of regexp text:
preg_match_all("/\[\[.*?\]\]/s", $text, $matches, PREG_SET_ORDER);
This must be replaced to:
preg_match_all(MEDIA_WYSIWYG_TOKEN_REGEX, $text, $matches, PREG_SET_ORDER);
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | media_2825653_fix-token-filter-9.patch | 557 bytes | Murz |
| |||
| #20 | media_2825653_fix-token-filter-8.patch | 555 bytes | Murz |
| #14 | media-wysiwyg-fix-token-7.patch | 1.1 KB | Murz |
| #6 | media-wysiwyg-fix-token-6.patch | 1.09 KB | joseph.olstad |
| |||
Comments
Comment #2
MurzComment #3
MurzAlso it conflicts not only with specific module, tokens, but with all plain texts using double square brackets, also in html tag arguments, for example:
1.
<div class="tablechart" data-height="200" data-legend="top" data-tabledata="[["Search","Visits"],["Yandex",47],["Google",29]]" data-title="" data-type="PieChart" data-vaxis="true"> </div>2.
<p>Here is [[problem demonstration]] for media wysiwyg module.</p>Comment #4
joseph.olstadComment #6
joseph.olstadreroll of original patch by @Murz
Comment #7
jmdorian CreditAttribution: jmdorian commented#6 works for me, thanks!
Comment #9
joseph.olstadlooks good, its committed to dev. If no complaints then it'll make it into 7.x-2.0-beta6 likely in a few weeks
Comment #10
sylus CreditAttribution: sylus commentedI have traced an issue to this commit:
http://cgit.drupalcode.org/media/commit/?id=db5b708c50a43d2986d85037ebf8...
Immediately before this commit doing a fresh install of a distro all media images in wyswyg were working, then when leveraging this commit, some images but not all are no longer working. Seems like it is related to this issue:
https://www.drupal.org/node/2828665
Comment #11
sylus CreditAttribution: sylus commentedComment #12
joseph.olstadComment #14
MurzSeems that sometimes fid escaped with
'instead of", so I extend regex for matching both variants.Comment #15
MurzComment #18
junaidpv"fid" does not come always as first property in media tag. That how I see many media tags in one of our production sites. So, patch from #14 is not enough.
Comment #19
Murz@junaidpv, can you provide some examples in which cases media tags not starting with "fid"?
I review php code in all Media module and can't find places in which it can add tokens, starting not from "fid" text.
IMHO, best solution will be add "media" prefix for all media tokens - what do you think about this way?
Comment #20
MurzI have found another much easier way to fix this - replace token regex from:
to:
This is prevent media module for touching other tokens without media types. Same way works in
media/modules/media_wysiwyg/js/media_wysiwyg.filter.jsjs file:So think this is true way.
Patch is included.
Comment #21
MurzComment #23
MurzUpdated regexp for multiple Media tags
Comment #24
joseph.olstadSo if I'm understanding this correctly,
However, I have not yet noticed this myself.
@Murz, what is the test scenario? How can you reproduce this scenario? I am not sure how to reproduce or test your patch.
Comment #25
MurzTest scenario is very simple: try to add any text with double square brackets in text, for example:
Media module remove all items with double square brackets. My patch will fix this and Media module will touch only tokens that contains
"type":"media"text.Comment #26
joseph.olstadany volunteer reviewers?
Comment #29
joseph.olstadanyone interested in trying the latest enhancement by @murz , you can download a copy of 7.x-3.x-dev , once it gets some mileage, then it'll get merged or cherry picked back into 7.x-2.x. Going to try to be extra vigilent about what makes it into the 7.x-2.x branch.
Comment #30
joseph.olstadThis (media_2825653_fix-token-filter-9.patch) is included with the 7.x-3.x-dev release , if you like it after review, the above fix can be merged into the 7.x-2.x branch and scheduled for the next release.
Comment #31
joseph.olstadFixed in 7.x-3.x ,
minor version target for (needs merge to) 7.x-2.x
Comment #33
joseph.olstadThis was just merged into 7.x-2.x and is included in release 7.x-2.6
Comment #34
joseph.olstad