Problem/Motivation (as of #137)
Media supports inclusion of files into WYSIWYG areas. WYSIWYG editor renders files using the same view mode as it will be later used in rendered entity display. This is sometimes appropriate, but it can lead to inadequate results if a non-image formatter is used. This commonly surfaces with document and audio files that might have a text link. Once inserted, these are no longer editable from within WYSIWYG.
This issue tries to solve that problem by introducing the ability to configure the default view mode used in WYSIWYG itself. This would still allow the usage of non-image formatters for rendered entities, but could be replaced with some sort of a default image formatter (e.g. maybe a thumbnail of the PDF) in WYSIWYG.
Proposed resolution
- Merge functionality from view_mode sub-module into media_wysiwyg.
- Store WYSIWYG View Mode configuration in a db table.
- Make WYSIWYG View Mode exportable via Features.
- Add a UI field to entity file display form to disable view modes in WYSIWYG. (admin/structure/file-types/manage/%file_type/%file_display)
- Add a UI field to entity file file type form to set a default view mode in WYSIWYG. (admin/structure/file-types/manage/%file_type)
Remaining tasks
Reviews and testing of #166.
User interface changes
- adds configuration for wysiwyg view modes
API changes
n/a
Original report by Devin Carlson
An offshoot of #1283844: [meta] Improve WYSIWYG integration, Media cannot currently embed non-images using WYSIWYG. The current WYSIWYG code requires an image to be output or it will not allow you to submit the "add media" form after selecting a view mode for a chosen piece of media.
The reason the embedding of non-image media is broken is because the current WYSIWYG code tries to render the chosen media using the media player that is currently configured for the selected view mode and place the resulting HTML into the editor. In many cases this HTML will not include an image.
Media places a "token" into the WYSIWYG editor to represent a file. Similarly, it should also display an image "token" to represent the file instead of inanely trying to render the actual media. Custom plugins for Media may output iframes, divs or simply hundreds of lines of HTML; all things that don't belong directly in the WYSIWYG.
In order to allow embedding of non-image media, I propose that Media does the following:
- Assume that a file displayed using the "preview" view mode outputs an image
- Use whatever image is outputted by a file's preview view mode as a "token" to represent the media when it is in a WYSIWYG editor
- Display the file appropriately when outside of WYSIWYG
Comment | File | Size | Author |
---|---|---|---|
#208 | interdiff_205-208.txt | 477 bytes | joseph.olstad |
#208 | media-custom_view_modes_for_wysiwyg-1792738-208.patch | 34.86 KB | joseph.olstad |
Comments
Comment #1
Devin Carlson CreditAttribution: Devin Carlson commentedA patch to implement the change.
Comment #2
brunodboWorks great. After applying this patch, I was able to embed a pdf file using the media browser in the WYSIWYG editor. In the WYSIWYG editor field, the file appeared as a placeholder media icon; when viewing the node, it displayed as a pdf icon with a link to the file.
One question (which probably should be dealt with in a separate issue): would it be possible to use smaller placeholder icons in the WYSIWYG editor? They look very big right now in the editor, especially when embedded in a paragraph of text.
Comment #3
RobW CreditAttribution: RobW commentedIf the final output for a PDF is an image in this case, why not use that same image in the WYSIWYG?
Comment #4
Devin Carlson CreditAttribution: Devin Carlson commentedRobW: It does. You can enable a formatter that returns an image for the preview view mode and it will be used inside of the WYSIWYG.
For example, if you enable the "Image" formatter for the "preview" view mode of the "Image" file type, an image preview will be inserted inside of the WYSIWYG instead of a filetype icon.
This also works for other filetypes. If you want to see previews of PDF files inside of the WYSIWYG instead of file type icons, install an appropriate formatter that returns an image, such as PDFPreview (with some slight modifications) and Media will insert a preview image of the PDF file into the WYSIWYG.
brunodbo: if people are relatively satisfied with this method, I would be happy to work on creating preview formatters for different file types or at least providing better (more appropriately sized) default file type images for files embedded with WYSIWYG.
Comment #5
Devin Carlson CreditAttribution: Devin Carlson commentedJust took a look through the issue queue, this would fix/address concerns/improve:
Comment #6
ParisLiakos CreditAttribution: ParisLiakos commented#1: use-preview-view-mode-with-wysiwyg-1792738-1.patch queued for re-testing.
Comment #7
Devin Carlson CreditAttribution: Devin Carlson commentedUpdated patch to use the "preview" view mode instead of "media_preview" now that #1051090: Revamp file view modes: migrate media_small to teaser, media_large to full, media_preview to preview; deprecate link & original and #1296268: Add Preview and Teaser view modes are fixed.
Comment #8
mrfelton CreditAttribution: mrfelton commentedCan not insert PDF files even with this patch. I can't even select them in the media popup. Note, I have enabled the Document filetype for use with Media WYSIWYG at /admin/config/media/browser. I can see the PDFs in the media WYSIWYG popup. But, I can not select, nor insert them.
Comment #9
Devin Carlson CreditAttribution: Devin Carlson commented@mrfelton, you'll need to make sure that you've enabled a formatter that returns an image for each file type's "preview" display mode.
To embed a pdf file, try enabling the "large filetype icon" formatter for the Documents file type.
I'm mainly interested in seeing if this idea has any support. I suppose I should look into doing something like enabling the "large filetype icon" formatter for every filetype's preview view mode by default. Or at least changing the fallback/default display from "generic file" to "large filetype icon".
Comment #10
martins.bertins CreditAttribution: martins.bertins commentedSetting the "large filetype icon" formatter for "Document" file type "Preview" display mode really fixes the problem for selecting and embedding pdf and other documents from media library. Just the thing that I needed.
But I ran into another problem. First we had Media and File Entity module versions 7.x-2.0-unstable3, I updated both of them to latest dev and added #7 patch to fix the pdf embed problem. So far so good, but now the problem is that other document files that were added to media library before module upgrade still are not selectable and use the old display style. There is no problem with images added before module upgrade.
Screenshot: https://skitch.com/becizzz/e2t65/luo-sivu-innoomnia.fi
Comment #11
martins.bertins CreditAttribution: martins.bertins commentedalso changing the status
Comment #12
Devin Carlson CreditAttribution: Devin Carlson commented@martins.bertins your issue is caused by #1496624: Provide an upgrade path for media types to file_entity types. I'd suggest testing the latest patch in that issue and see if it fixes the problem (and please report your feedback, it would be much appreciated).
Comment #13
martins.bertins CreditAttribution: martins.bertins commentedThanks Devin for pointing to that issue. Used the patch from it and now got everything working.
Comment #14
Rob_Feature CreditAttribution: Rob_Feature commentedIf I understand this correctly, all of this "just use the preview mode" stuff works on every file type...INCLUDING on images?!?
I'd say this behavior should work on every other type of file, but leave images alone to work how they do now. It's often very important that we see the actual output size of an image when inserting it into content. It allows us to determine how much space it's sucking up and give a reasonable preview of what we're embedding. Always inserting the same size image preview seems like a really bad idea all the way around.
If possible, I'd suggest this patch target all file types except images...but leave images alone.
Comment #15
Rob_Feature CreditAttribution: Rob_Feature commentedHere's a patch that basically does this same thing, but only if it's not an image file type (for those we use the existing behavior).
Comment #16
Kazanir CreditAttribution: Kazanir commentedWe really need to find a way to merge this with rootatwc's patch here: http://drupal.org/node/1451316
This patch pre-selects the Preview view_mode for use inside the WYSIWYG editor, while rootatwc's patch above renders the output as it will actually output on the page. Sometimes one or the other is preferable and it would be ideal to be able to pick between them somehow. This would elegantly solve the problem that Rob is hacking on above.
OTHER than that issue this patch works great and is very useful for being able to embed documents and other image-less content in the first place.
Comment #17
Devin Carlson CreditAttribution: Devin Carlson commentedFor anyone following this issue, I've setup a sandbox project which enables you to choose a select a view mode to use, per file type, when rendering media inside of the WYSIWYG editor. Combined with Entity View Mode, this should be able to accomplish what Rob_Feature described in #14.
Project: Media WYSIWYG View Mode
If you're unfamiliar with applying patches, you can use the sandbox project to test out this approach to WYSIWYG and see what you think.
Comment #18
Rob_Feature CreditAttribution: Rob_Feature commentedI've tested Devin's Media WYSIWYG View Mode and I REALLY think this is the future....a couple small tweaks (which he said he's doing today) and it's exactly what's needed.
I propose closing this issue in favor of moving in the direction of that module...
Comment #19
DamienMcKennaAn updated version of #15 that a) adheres to the coding standards ;) b) has only one call to media_get_file_without_label().
Comment #20
DamienMcKennaThe separate sandbox brings up the question of: where do you draw the line between functionality to be included in the core module vs a separate module? The functionality is quite small, most of the code could be dropped if the functionality was just rolled in as an option on the file display settings.
Comment #21
ParisLiakos CreditAttribution: ParisLiakos commentedI think this is a very good idea..i tried today to insert some non standard filetypes to wysiwyg and if i was selecting displays which are not standard..eg not generic file...there was clearly a problem..this approach fixes this, but it somehow does not feel right to me..
I am gonna think about it a bit more in the next few days.
Comment #22
mrfelton CreditAttribution: mrfelton commentedHere is the same patch (I think), updated to work with latest media code.
Comment #23
ShaunDychko CreditAttribution: ShaunDychko commentedPatch works for me, thank you!
Comment #24
Anonymous (not verified) CreditAttribution: Anonymous commentedThanks for the patch - working great for me.
Comment #25
ParisLiakos CreditAttribution: ParisLiakos commentedi think we should discuss this a bit more..this approach feels very aggressive when embedding stuff that would otherwise work when embedding..eg file link, not just images..i would also love to hear other people opinions, especially other maintainers..
instead of whitelisting stuff that can be displayed in a wysiwyg editor in media module, maybe we should introduce a new property for formatters, eg sth like
'wysiwyg_view_mode' => 'preview'
that are known not to work inside wysiwyg..
what do you think?
Comment #26
mrfelton CreditAttribution: mrfelton commentedI think something like that makes sense @rootatwc. The more control that individual formatters have over the specifics of their wysiwyg integration the better. Media should not really assume anything one way or the other, although sensible defaults are good.
Comment #27
SebCorbin CreditAttribution: SebCorbin commented#22 Patch solve PDF linking issue here
Comment #28
Rob_Feature CreditAttribution: Rob_Feature commentedI'm getting nervous, honestly, that no one is looking at Devin's Media WYSIWYG View Mode module, which really solves all of these problems perfectly....PLEASE try it and feedback to the Media team. I now find that I can't use Media/WYSIWYG without it and I really REALLY think it (or parts of it) should be considered as being added to Media core....
http://drupal.org/sandbox/DevinCarlson/1823634
PS: Note a small bug creeped in recently but it's still an essential for me: #1886234: Restrict View Modes Broken
Comment #29
rooby CreditAttribution: rooby commentedI have installed the sandbox at http://drupal.org/sandbox/DevinCarlson/1823634 and with pretty much no configuration required I could do what pretty much every client of mine wants...
Add a pdf file via WYSIWYG and have it embed inline as a link to the file.
Fantastic, and it works in a logical manner.
I have not reviewed very thoroughly yet though.
Comment #30
ParisLiakos CreditAttribution: ParisLiakos commentedtagging
Comment #31
cosmicdreams CreditAttribution: cosmicdreams commentedWhat is the way forward? Package up Devin's work as a patch?
Comment #32
Dave ReidThe sandbox looks good, but I think we can still go with a simplified approach that can be easily extended. Why not make which view mode is forced to be in the WYSIWYG a variable, with preview as the default value, but not exposed in the UI.
Basically implementing https://gist.github.com/3012194 but in the Media module.
Comment #33
gmclelland CreditAttribution: gmclelland commentedYesterday I tried out the Media WYSIWYG View Mode module at http://drupal.org/sandbox/DevinCarlson/1823634 and I really liked how it worked.
I just wanted to point out that it also provides another big feature that enables you to see an ajaxified preview of the view mode selector after choosing a media item from the media browser before the media is inserted into the wysiwyg.
@Dave Reid's suggestion sounds good too.
I think it would be beneficial to set a message when editing the "Preview" mode at "Manage File Display" that says something like this view mode is used for previewing non-image media that is inserted into a WYSIWYG editor. This could help newcomers who question what the view mode does.
Comment #34
Rob_Feature CreditAttribution: Rob_Feature commentedIt's important to be able to select, per file type, whether we use a special wyiswyg view mode or not (in the UI). For example: I always want to use the "WYSIWYG preview view mode" for audio, video, and documents. But I NEVER want to use it for images because I want to truly see the view mode (ie imagestyle) for images...forcing these to use a single view mode would be extremely unfriendly.
Comment #35
RobW CreditAttribution: RobW commentedChoosing in the UI seems a bit overkill. IMO this is a place where decisions trump options; media provider modules deciding in code seems like it would create the best Drupal sitebuilder/user ux.
Comment #36
gmclelland CreditAttribution: gmclelland commentedI just wanted to point out that the Media WYSIWYG View Mode at http://drupal.org/sandbox/DevinCarlson/1823634 allows the ability to select which view mode to use on the media item that is going to be inserted into the WYSIWYG editor. If the media item is an image, the module will initially display the image above the view mode selector in the media browser modal dialog.
Should that initial preview use the "Preview" view mode as well? Right now it shows the image at full size see #1901114: Default view mode for previews initially until someone chooses another view mode and it updates the preview via ajax.
So my question is where will the "Preview" view mode be used? For clarification, where is it currently used now?
- in the WYSIWYG editor (except the image file type)
- possibly above the view mode selector when using Media WYSIWYG View Mode module?
Should these use the same view mode?
Also note that the Media WYSIWYG View Mode module also adds a "WYSIWYG" view mode.
Comment #37
Rob_Feature CreditAttribution: Rob_Feature commentedgmclelland you're correct, and keeping this stuff in the UI is what I advocate for. There's such a mix and match of how this could be used (and it's useful works great) that stripping it out seems strange.
RobW, I'm not trying to be difficult, but have you used Devins module in a real world circumstance? It's SOOO perfect in how it addresses all the issues that I fear if it goes away it makes media/wysiwyg actually LESS flexible than it is now.
I guess all I'd advocate for would be to do this in a way that something LIKE Devin's module could continue to exist as a separate contrib...it's a crucial piece to this puzzle. (basically I have no problem either way, as long as something like Devin's module COULD exist...it doesnt matter to me if it's in media core or not)
Comment #38
rooby CreditAttribution: rooby commented+1 for #37
Comment #39
RobW CreditAttribution: RobW commentedRe: #37, I have no strong feelings against it, I just prefer to make the choice in code. If others find it useful, go for it.
Comment #40
Kazanir CreditAttribution: Kazanir commentedDevin's module is the way to go. Simplifying it down to a toggle of a hard-coded preset view mode or making the choice in code is not a good UX for the average Drupal site administrator -- one of the audiences that benefits MOST from a module like Media. This is doubly especially since so many other site-administrator-level modules in D7 (Views, Panels, Display Suite, Media, et al) rely on picking view modes and configuring them to do what you want. If I had the coding chops I would even expand Devin's module to work like this:
[File Bundle WYSIWYG Configuration]
[Checkbox] Enable $view_mode when inserting via WYSIWYG? If yes, enable [Select List of $wysiwyg_modes] Replace $view_mode with $wysiwyg_mode while inside editor?
[...] for each $view_mode
This would allow the site administrator to replace problem-causing view modes (such as video players or colorbox links) with image-only view modes while inside the editor frame, but not mess with "friendly" view modes at the same time.
Comment #41
RobW CreditAttribution: RobW commentedIn the example in #40, it seems to me that replacing a malfunctioning wysiwyg insert with another vm is the responsibility of the media provider module maintainer. If an insert doesn't work it's a bug, not config. As a person who both builds modules and builds sites, I don't want to do any more config, especially where choices don't offer any significant improvement for the majority of people.
But there seems to be consensus, so do yo thang.
Comment #42
rooby CreditAttribution: rooby commentedI can definitely see both sides.
I like the configurability of the sandbox, however as an example, in the case of adding a pdf inline, if it defaulted to uploading the pdf and inserting it in the text as a link to the pdf (what I imagine would be the 99% use case for pdfs) I would be even happier and would not need to touch the config 99% of the time.
So I definitely see where RobW is coming from too.
Comment #43
Rob_Feature CreditAttribution: Rob_Feature commentedI want to suggest again that, if there's some big proponents of just hard coding this setting in code that's ok....IF (huge 'if' here) it can be overridden easily by an external module such as WYSIWYG View Mode.
The way I see it, this has to be fixed, somehow, in Media core. The module needs to be able to embed all these different types, in some way, by default. This means something has to go into media to make that work better by default.
But, what we're talking about here is exactly how that's added. If there's truly proponents for a hard coded preset, I'd suggest going in that direction but always keeping in mind that another module MUST be able to hook in and do what Devin's mod does in the UI.
Something like that seems the best of both worlds if there are truly a bulk of folks who advocate for no UI. On the other hand, if the mass agrees that a UI component makes the most sense, I certainly throw my weight behind that direction.
Comment #44
gmclelland CreditAttribution: gmclelland commentedrelated issue #1038884: Display image format preview on field insert as well as wysiwyg insert format page
Comment #45
Dave ReidCan we add the functionality of the sandbox without exposing the UI of it?
Comment #46
slashrsm CreditAttribution: slashrsm commentedIssue summary made me think that it is currently only possible to include image files in WYSIWYG. This is however not true. Using latest -dev of Media and File entity I was able to include PDF document that used Embedded Google Docs Viewer for display. Everything worked OK (GDoC viewer on rendered node and iframe placeholder in WYSIWYG).
Since I agree it is not always the best solution to use same formatter for wysiwyg and rendered node, I still believe that this should be the default behaviour (for similar reasons that were mentioned in #42).
Devins sandbox implements very handy feature, which could be included in Media or it could also live in contrib. It is up to us to decide about this. I am convinced that we also need to provide UI for it if we decide to pull it in Media core. As already mentioned; Media is frequently used by site builders who rely on UI for configuration. Offering good experience for them will definitely speed up the adoption of Media IMO.
Anyone interested in IRC/Hangout meeting about this issue where we could hopefully find and agree on a solution for this?
Comment #47
azinck CreditAttribution: azinck commentedslashrsm: count me in for an IRC or hangout get-together on this issue.
Comment #48
slashrsm CreditAttribution: slashrsm commentedI suggest that we discuss this on next regular meeting: http://groups.drupal.org/node/286568
Comment #49
slashrsm CreditAttribution: slashrsm commentedThere was a decision made about this issue at yesterday's meeting.
We are going to pull Devin's module in Media. We only need to change configuration stuff a bit. We'd like it to be save in a file type (and being exportable) instead of having things is separate variables.
Let's do it! We have just some small bits left to reach alpha1.
Comment #50
slashrsm CreditAttribution: slashrsm commented@Devin Carlson: You are still marked as assignee for this issue. Are you still interested in preparing a patch for this?
Comment #50.0
slashrsm CreditAttribution: slashrsm commentedUpdate issue summary
Comment #51
chrisschaub CreditAttribution: chrisschaub commentedAny movement on this integration of Devin's sandbox module?
Comment #52
chrisschaub CreditAttribution: chrisschaub commentedI could roll his module as a patch to 2.x dev if needed this morning. I think the only discussion was to move the variables to exportables? Otherwise a straight port -> patch?
Comment #53
chrisschaub CreditAttribution: chrisschaub commentedAfter some more digging, wouldn't it be better for sites to use ...
http://drupal.org/project/entity_view_mode
just make it a dependency of media? That way users have total flexibility?
Comment #54
azinck CreditAttribution: azinck commentedcschaub -- this issue is about setting rules around which view modes are allowed to be used in WYSIWYG as well as what view mode is used to replace the macro Media injects into the text area when using the WYSIWYG. The needs here go beyond what Entity View Mode provides.
Comment #55
chrisschaub CreditAttribution: chrisschaub commentedI don't see much difference at first glance, I tried both approaches. The only differece I saw was the ability to restrict which view modes are shown. In the end, the module is just creating a view mode. I'll dig deeper to see what I'm missing.
Comment #56
rooby CreditAttribution: rooby commentedEntity view mode only allows you to create new view modes.
This issue is about using those view modes when inserting media via the WYSIWYG.
Use both together for happiness.
Comment #57
Dave ReidComment #58
aaron CreditAttribution: aaron commentedThis is a straight add of the sandbox module.
Comment #59
slashrsm CreditAttribution: slashrsm commentedEmpty patch....
Comment #60
gmclelland CreditAttribution: gmclelland commented@aaron - Can you attach the correct patch?
Comment #61
aaron CreditAttribution: aaron commentedwoops, sorry about that!
Comment #62
supradhan CreditAttribution: supradhan commentedIs this patch is same as sandbox project: https://drupal.org/sandbox/DevinCarlson/1823634?
Comment #63
gmclelland CreditAttribution: gmclelland commented@ClsSharp - I haven't tested the patch in #61 yet, but it looks like the code is the exact same except now it lives under the Media module as a separate Media WYSIWYG View Mode module.
Comment #64
Dave ReidMoving to a beta blocker. I don't think feature requests should hold up a long-awaited alpha release.
Comment #65
Dave ReidComment #66
Dave ReidComment #67
gmclelland CreditAttribution: gmclelland commentedJust tested #61 on a fresh install. Everything is working great.
These recommendations could be added in followup issues:
1. Maybe change the word "restrict" to "disable" at admin/config/media/wysiwyg-view-mode?
2. Add a default view mode selection per file type - since the view modes are sorted alphabetically, someone might have a view mode with a large image. You don't want that person to always see a large image as the default selection.
I tested with the latest media and file_entity-2.x-devs and with Ckeditor and TinyMCE.
Hope that helps
Comment #68
gmclelland CreditAttribution: gmclelland commentedNevermind, all is working well.
On an older install of file and media 2.x-dev, it would automatically display the viewmode.
On the newer versions of file and media-2.x-dev, it shows just a link with the file name by default and doesn't update the preview until you actually make a selection. Very nice
Here is a screenshot.
Comment #69
aaron CreditAttribution: aaron commentedCommitted to http://drupalcode.org/project/media.git/commit/7d6a333
Comment #70
Dave ReidI don't see the changes that were requested in comment #49 were actually made before this was committed.
Comment #71
Dave ReidAlso I think the intent was to merge this into media, and not actually have it as a sub-module, but that may not have been very clear.
Comment #72
aaron CreditAttribution: aaron commentedSorry I jumped the gun on that. I will roll a new patch soon.
Comment #73
bgilhome CreditAttribution: bgilhome commentedI added a default view mode per file type to Devin's sandbox at https://drupal.org/sandbox/DevinCarlson/1823634 - see patch and comment at https://drupal.org/node/1901114#comment-7686315. Once the above patch is re-rolled I could upload a new patch from that.
Comment #74
aaron CreditAttribution: aaron commentedI have made some progress in the sense that I can now save the restriction state but I am having troubles figuring out how to load it again. There appear to be no hooks available during the file type load. I would appreciate if someone could help point me in the right direction. I am largely unfamiliar with the internal workings of the cTools module.
Comment #75
drupal_was_my_past CreditAttribution: drupal_was_my_past commentedCommit 7d6a333 reintroduced a bug from #2003810: When editing a file in the WYSIWYG, the "Display as" view mode selection does not inherit the default value where the Display As select form no longer inherits the default value. I've added a patch in comment #3, but I'm not sure if it is needed if @aaron is going to be reworking media_wysiwyg_view_mode module into the media module itself.
Comment #76
drupal a11y CreditAttribution: drupal a11y commentedHi All,
as Designer & Sitebuilder I see some issues:
Please keep in mind I still need to figure out the latest inventions and that maybe I currently have a wrong understanding cause I am not deeply involved in the progress of the past month.
Found these modules on that topic:
Comment #77
drupal a11y CreditAttribution: drupal a11y commentedOn my research & tests I also found this older issue by David Reid where support for "WYSIWYG Fields" was discussed: https://drupal.org/node/1664418
The ease of the "insert"-module combined with the media-browser by supporting this module would be really great.
If I can help somehow with Design-Drafts, Mockups, HTML/CSS please let me know.
Comment #78
drupal a11y CreditAttribution: drupal a11y commentedAnother D6 module which has a some UI/UX goodies on inserting images is https://drupal.org/project/img_assist
Comment #79
aaron CreditAttribution: aaron commentedmori, thank you for your input. However, it would be appreciated to open a new issue rather than hijacking an existing issue. That helps to keep things clear.
Comment #80
gintaras.r CreditAttribution: gintaras.r commentedSorry if this is a distracting question, but I'd like to know how these changes might integrate with the Spark project. As far as I can tell, Spark uses a CKEditor module as a wrapper instead of the WYSIWYG module. But WYSIWYG is a dependency for this new sub-module. A more general question might be, will site admins miss out on this functionality if they don't go through the wysiwyg module to set up their wysiwyg editor. Is Spark not following convention by doing that?
Comment #81
StevenWill CreditAttribution: StevenWill commentedAny progress on this issue?
Comment #82
fengtanFollowing #74: Could not find a way to hook into file_type_load() either. We found a workaround by altering the 'subrecords callback' from ctools, though there may be a better way to do this.
Attached is a patch that moves the config stuff from separate variables to file types (see #49).
We added some documentation while we were at it. Also, we removed the 'restrict' checkbox from the default view mode since it is always presented to the user, but we can change it if this was not expected.
Maybe this can be improved before we move it into media module.
Comment #83
aaron CreditAttribution: aaron commentedThanks for getting the ball rolling again, @Fengtan. This approach looks fine to me. I am setting the status to "needs review" so that we can get the testbot's thoughts on the matter. However, I think that we still need to move it all into the media module to be able to wrap it up.
Comment #84
aaron CreditAttribution: aaron commentedI'm not certain how the tag was removed. I'm adding it back in.
Comment #85
igorik CreditAttribution: igorik commentedAfter I manually applied patch I got this message:
Fatal error: Call to undefined function file_type_load_subrecords() in ...../sites/all/modules/media/modules/media_wysiwyg_view_mode/media_wysiwyg_view_mode.module on line 137
Comment #86
rooby CreditAttribution: rooby commented@igorik:
What version of the file_entity module are you using?
Comment #87
igorik CreditAttribution: igorik commentedI tried it now with latest media and file entity (7.x-2.0-alpha3) and still the same problem
Comment #87.0
igorik CreditAttribution: igorik commentedUpdate summary.
Comment #88
fengtanFollowing #83: Re-rolled the patch so the 'wysiwyg view mode' code lives in the media module. Hope it helps.
@igorik not sure how you got this message, file_type_load_subrecords() seems to be a valid function.
Comment #89
gmclelland CreditAttribution: gmclelland commentedRelated issue #2062659: Roll the media_wysiwyg_view_mode module into media module.
Comment #90
gmclelland CreditAttribution: gmclelland commentedComment #91
micbar CreditAttribution: micbar commentedComment #93
alisonFrom @gintaras.r's comment (#80):
My understanding was that D8 would be using CKEditor without the WYSIWYG "integrator" module, but the proposed fix requires said module. It doesn't seem like the best plan to me -- easy for me to say -- but, if it's the path you need/want to take, (a) I think this issue title/description should be edited to clarify that the thread/proposed fix is for integration with the WYSIWYG module (trying to discern "WYSIWYG module" vs. "WYSIWYG with a theoretical lowercase W" in issues/forums is so confusing); and, (b) I question whether the it should be in Media core, therefore making WYSIWYG a dependency of Media. (Or am I misunderstanding the intent of the proposed fix?)
I realize you've already given most if not all of this a great deal of thought, but I'm throwing in my two cents anyway.
Comment #94
fengtanLooks like file_type_load_subrecords() does not exist anymore in file_entity-7.x-2.0-alpha3.
Here is a new patch that might pass tests.
If anyone knows a kind of hook_file_type_load() that would be awesome.
Comment #95
micbar CreditAttribution: micbar commentedComment #96
Mirabuck CreditAttribution: Mirabuck commentedComment #97
Mirabuck CreditAttribution: Mirabuck commentedAdding #2142571: Spin-off WYSIWYG support in dedicated module / project as a related ticket as I suspect it has significant implications for this one. Given that #94 was based on the pre-2142571 module structure, it no longer works on top of the most recent dev and will need to be reworked.
Comment #98
Mirabuck CreditAttribution: Mirabuck commentedComment #99
Mirabuck CreditAttribution: Mirabuck commentedGiven what's happened in #2142571: Spin-off WYSIWYG support in dedicated module / project, what are the current plans for media_wysiwyg_view_mode module? Will it remain separate from media_wysiwyg (in which case patch #82 above may be close to what we need) or should it be integrated with media_wysiwyg (in which case we need a significantly refactored patch)?
Comment #100
fengtanRe-rolled patch so it applies against the current -dev.
Attached are a few screenshots of how it should look like.
Note that #2075123: Media browser inserting HTML tag rather than media tag makes the wysiwyg view mode ignored when using Firefox -- but there is a patch that seems to fix it.
Comment #101
fengtanComment #102
camdarley CreditAttribution: camdarley commented#100 works! Thanks.
But there is still 2 things that I don't understand:
- Why the default view mode is set to "full"? As mentioned in #1901114: Default view mode for previews it's not very easy to embed very large images.
- Should there be an option to display the image in the selected view mode, once embedded in the wysiwig, instead of a default view mode? It may be very confusing for users who chose "link" or "preview" and have a large image in their wysiwyg instead.
Comment #103
fengtanThanks for your feedback camdarley.
Comment #104
Mirabuck CreditAttribution: Mirabuck commentedI suspect patch 100 has introduced a markup issue of some sort. Without the module enable, media's modal window works fine for me. With it enabled, in chrome, the scrollbar disappears and the styles look a bit off. Turning off the view modes module restores regular the regular look and feel.Looks like this was an unrelated issue on my end. Works as expected for me currently. At first I didn't understand why we needed an extra WYSIWYG view mode in addition to the toggles and settings this module adds, but now I'm figuring it allows us to have an additional wysiwyg-specific view mode if none of the existing ones are appropriate.
Comment #105
aaron CreditAttribution: aaron commentedHere is a re-roll that integrates your work with the Media WYSIWYG module.
Comment #107
Mirabuck CreditAttribution: Mirabuck commentedWith additional testing it's become clear to me that the new config patch 100 adds is not currently featurizable.
Comment #108
iwayMan CreditAttribution: iwayMan commentedRe-rolled #105 by removing media_wysiwyg_view_mode module and references.
Comment #109
aaron CreditAttribution: aaron commentedIf the patch works, then we can put it in, and featurize it later.
Comment #110
dddbbb CreditAttribution: dddbbb commentedI agree with #109 providing it wasn't "featurisable" in the first place (I'm guessing it wasn't as this is new functionality).
Comment #111
Mirabuck CreditAttribution: Mirabuck commentedThe original sandbox module used a variable and could therefore be featurized via strongarm. When the data was moved into a custom table we lost this.
We need featurization pretty desperately on our end. iwayMan is most of the way through a reroll that adds it.
Comment #112
iwayMan CreditAttribution: iwayMan commentedHere's another patch. Contains everything from #108 and adds featurization of media_wysiwyg in new file includes/media_wysiwyg.features.inc.
Comment #113
Mirabuck CreditAttribution: Mirabuck commented#112 works for me as expected. It looks like it leaves part of the old view modes module in place, however, which we may want to remove.
Someone other than me should probably verify this given that iwayMan and I are working together on the same project.
Comment #114
Mirabuck CreditAttribution: Mirabuck commentedAfter working with this for a few more weeks a few things stand out to me:
Comment #115
Mirabuck CreditAttribution: Mirabuck commentedThe attached patch takes what we had in 112 and provides an option to not use a WYSIWYG view mode. This seems to work as expected. What I can't figure out though is how to ensure that when WYSIWYG view modes are disabled images get rendered within the WYSIWYG appropriately. Seems like the following snippet returns receives 'default' as a view mode even when a user has chosen another view mode in the WYSIWYG modal--maybe it doesn't get updated in PHP because the modal stuff is happening in JS? I'm not certain what goes on in the background with the modal stuff.
Removing the snippet altogether seems to have no effect.
Comment #116
fengtanWe found an issue when displaying images using a view mode with a larger width/height than the wysiwyg mode. The image would always be displayed using the wysiwyg mode.
Rolled a new patch with a possible fix.
Comment #117
igorik CreditAttribution: igorik commentedHi, is this already implemented in Media module, or not yet?
Comment #118
bneil CreditAttribution: bneil commentedigorik - the patch above is not included in the module yet - please help us test it and provide feedback so we can include it soon.
Comment #119
Mirabuck CreditAttribution: Mirabuck commentedThis patch (116) works, but doesn't fix the issue with images always being rendered with the default view mode in the WYSIWYG.
It also pushes preview images back into the top of the modal (a departure from the current dev behaviour).
Comment #120
fengtanAttaching screenshots. Hope it helps.
Comment #124
Mirabuck CreditAttribution: Mirabuck commentedPatch 120 works for me in terms of allowing file types to be properly excluded from view modes.
Nor sure about the UX stuff. Is the desire to have an unchanging preview image in the LHS margin or to have preview images change based on what display mode's been chosen?
If an unchanging image image desired, we probably need to tweak the patch a bit to accomplish this. If a changing image is desired, putting it back on top may make more sense as otherwise the form gets pushed over significantly with larger sized images.
Comment #125
meecect CreditAttribution: meecect commentedSorry if my question is too unrelated to this issue, But I noticed that even if I choose to show a pdf insert, for example, and choose to display it as 'link', I get a link to tfile with a small filetype icon, but the user is unable to change the link text.
I'm basically trying to recreate the old 'insert' module function here and I have numerous users that really preferred that workflow to the media centric workflow.
There, they would upload their file and attach it to the document, and then insert a link to it in their content wherever they wanted, and then edit the link text to fit their needs.
Of course, there are limitations to that workflow like inevitable broken links, multiple copies of the same attached file, etc, but it was simple and it worked.
Using the media approach and the work done in this thread is almost there, and does allow one to customize how the content will be rendered, but still within the limitations of the view modes.
I guess a solution would be to add a 'title' field to the file entity and create a view mode for it that rendered as the 'title' with an href to the file URL, but even then, the title would stay the same across the site where the entity is referenced.
If this is too off-topic, can anyone suggest a solution, or another item in the issue queue that may fit my use-case? IE, being able to easily insert a link to content you have uploaded and control the text of the link?
Thanks,
Comment #126
azinck CreditAttribution: azinck commented@meecect: Add the title field to the file entity. When you insert it in the WYSIWYG you get the chance to edit the field and that edit is preserved as a local override of the global value.
Comment #127
jmuzz CreditAttribution: jmuzz commentedI've been having some success with the latest dev, but after applying #120 here embedding an image no longer includes the fields attached to the image entity, it is just an image tag now.
Comment #128
delmarr CreditAttribution: delmarr commentedMaybe someone can verify this.
When adding an image via view-mode into the wysiwyg the image displays with the appropriate size.
There seems to be an issue when changing the existing view-mode. If you highlight that image and change the view mode via the media button the view-mode class gets appended to the img tag, but doesn't remove the initial view mode class. Which on saving displays the image size correctly, but shows up incorrectly in the wysiwyg.
This is currently working in the 7.x-2.x (dev)
Comment #129
bradezone CreditAttribution: bradezone commentedGlad to see work is being done on this. Just thought I would chime in and say that I tested the latest --dev versions of media and file_entity. I also had to update media_youtube to the dev version. Once everything was in place, I can now insert PDF's but several other things seem messed up, such as youtube no longer inserting the actual video (just a placeholder image), and images not being tied to specific display formats. (I don't even have the "file types" option anymore in config, where I set the file fields and display fields for the specific views like preview/default/teaser etc.)
So sadly it doesn't seem like a workable solution for me right now, when all I REALLY want is just the ability to insert PDF's inline with WYSIWYG (using CKEditor BTW).
Comment #130
heddnHere's a re-roll of #120. Me thinks some things are missing in it. But I wanted to get the re-roll done before I start changing anything.
Why alter the form? Why not add to the original form in pages.inc?
Why alter media_wysiwyg_token_to_markup? Why not change add directly to the original callback?
Why not change media_wysiwyg_get_wysiwyg_allowed_view_modes directly? Why use an alter?
Comment #132
heddnLet's see how this fares.
Comment #133
heddnHmm, the interdiff didn't work. All the alter functions from .module where inserted into the right places.
Comment #134
heddnThe latest patch picks up two things.
Comment #137
heddnThe placement of the view_mode logic inside media_wysiwyg_token_to_markup() caused a regression in #2062721: Add a white list of file fields that can be overwritten when the file is added in the wysiwyg. I've moved the logic inline into the function.
Also, the reason that tests failed in #134 were because the view_mode sub-module deletion got dropped from that patch. This patch also fixes that.
Comment #139
heddnComment #140
heddnComment #141
heddnThe view mode wasn't getting re-built correctly for images and content that doesn't have a default WYSYWG view mode defined. This patch restores the mechanism that was used previously.
Comment #143
heddnre-roll of #141.
Comment #144
blasthaus CreditAttribution: blasthaus commentedPatch in #143 is working for images, but that's all I'm able to insert into ckeditor. Any other file types, like pdf or a simple txt file will only display the file icon with no surrounding markup. Is this issue a part of a larger effort to make it possible to insert any file type with all the markup from a view mode or just img tags?
I've identified my particular problem and it has to do with the fact that ckeditor's own (now deprecated) "media" plugin using the same namespace is getting invoked instead of that of media module. Perhaps the file media_wysiwyg.ckeditor.inc which declares the hook_ckeditor_plugin() is not being found. I'm going to open a new issue on this specific bug since there is an error on that hook, but as this thread has several mentions of a similar issue, it's worth noting.
Comment #145
heddnre: #144
This is part of a larger effort to allow non image stuffs to be inserted into WYSIWYG.
This latest patch fixes an issue where media_wysiwyg_get_file_without_label() was being needlessly called twice.
Comment #146
heddnSome stuff from another patch slipped into that last patch #145. Let's try again.
Comment #147
klokie CreditAttribution: klokie commentedThe patch in #146 worked for images but didn't preserve the view mode choice from the overlay. The attached patch should fix that.
Comment #148
heddnre: #147
I'm curious, why are you using this for images?
Comment #149
klokie CreditAttribution: klokie commentedre: #148, it's my understanding that the purpose of this issue is to allow an editor to embed a media object in a WYSIWYG field. That's what the patch in #146 does, but it doesn't preserve the view mode (since it was being incorrectly passed from Javascript to
media_wysiwyg_format_form()
.I fixed that issue in the patch in #147, although in my testing it only works for embedding images - PDFs an other non-image media embedded in "Link" view mode are simply replaced by a thumbnail of that media type (without the link). I wasn't able to find out where that was happening - could be in either JS for this module or for Ckeditor.
Comment #150
blasthaus CreditAttribution: blasthaus commented@klokie
This was precisely my experience. I followed up with these 2 issues but silence so far.
media_wysiwyg.filter.js is not always loaded when using CKEditor
CKEditor plugins registered by hook does not include "media" module
Comment #151
Danny EnglanderI just tried the patch in #147 and could not apply it using
git apply
. The error was:So I then used the old method:
patch -p1 < media-custom_view_modes_for_wysiwyg-1792738-147.patch
... and it seemed to apply on several files except for this message:
File modules/media_wysiwyg_view_mode/media_wysiwyg_view_mode.info is not empty after patch, as expected
After that I saw there was a db update:
... ran that in drush and cleared cache. Now there's no longer a page at:
admin/config/media/wysiwyg-view-mode
Am I missing something?
Comment #152
Danny EnglanderI see, it looks like the setting is per file type now such as
admin/structure/file-types/manage/MYTYPE/edit
so that's a good thing. That being said, I expected my custom view modes that I created in a module to show up in the select list but they do not.Comment #153
heddnre: #147 & #150
I'm not seeing that the view mode is getting lost. The way this patch works is it adds the option to specify a default view mode for use in WYSIWYG @ admin/structure/file-types/manage/%file_typ/edit. This should be the default view mode used when inserting into WYSIWYG. If you are loosing the selected view mode, that is probably a different issue.
re: #151:
Are you applying the patch against the latest 2.x dev branch?
Comment #154
heddnI've updated the issue summary so hopefully the use case for this issue is easier to understand. I don't think that the patch in #147 is necessary. I'd recommend using #146.
Comment #155
heddnComment #156
Danny EnglanderOk I will try #146. I think I was expecting to see custom view modes that I've created with my own module (using
hook_entity_info_alter
) show up here but I don't think that's the case. I am guessing I need to do something extra for that to work so it's probably outside the scope of this issue. Thanks.Comment #157
heddnFor those interested, here's the list of patches I've got installed:
#2192391: Default file entities are not exportable by features (Sibling Issue)
#2348439: Allow dynamic WYSIWYG markup
#1792738: Allow custom file view modes for WYSIWYG display
#2062721: Add a white list of file fields that can be overwritten when the file is added in the wysiwyg
#2104193: Default file entities are not exportable by features (Media File Entity Overridden)
#2194821: Embedded media objects should honor display suite settings
Comment #158
mansspams CreditAttribution: mansspams commented#146 conflicts with #2062721: Add a white list of file fields that can be overwritten when the file is added in the wysiwyg as it also declares update_7201.
Comment #159
heddnre: #158: The hook_update will get rewritten if/when one of these gets committed during the next re-roll.
Comment #160
mansspams CreditAttribution: mansspams commentedAttaching patch with bump to update_7202.
Comment #162
heddnre #160: this assumes that the other issue will get committed first. Which might not happen. It is best to leave the hook_update_N as-is from #146. And if you use the patch on your own site, to remove the logic from the hook_update and include in a custom module's for your site. Until a hook_update gets committed, there's no saying with will get committed first. Or even another patch entirely, maybe even one that has security fixes included. And if you haven't removed the patches hook_update... then the other committed patches logic won't ever run.
To get this thing committed, we need testers and folks to RTBC #146. So let's do that instead of rolling needless patches. And leave it to the committers to renumber hook_updates.
Comment #163
mansspams CreditAttribution: mansspams commentedTesting setup as in #157 and everything seems to work well. Only question is about rendering of image + fields in WYSIWYG, image size is right, but fields are not rendered as defined in view mode. After saving tempate gets used and everything is fine. Now Im not sure anymore which patch is responsible for that.
Comment #164
heddnRe-rolling #146 as it no longer applies against HEAD. @mansspams, can you re-test and mark it RTBC?
Comment #166
heddnMissed a file in the re-roll.
Comment #167
heddnComment #168
mansspams CreditAttribution: mansspams commentedI wonder if .install file needs hook_install to trigger update. Something like...
Comment #169
heddnre #168: All the steps in the hook update get run in an install by default, or don't need to be run. What part of it needs running?
Comment #172
Rob C CreditAttribution: Rob C commentedpatching file modules/media_wysiwyg/includes/media_wysiwyg.filter.inc
Hunk #1 FAILED at 168.
Needs a minor reroll.
Comment #173
mstrelan CreditAttribution: mstrelan commentedReroll of #173.
Comment #174
mstrelan CreditAttribution: mstrelan commented#173 accidentally removes a + character.
Comment #176
heddn@mrfelton, since you ran a re-roll here, I hope that means you are using the code? Are you able to mark this RTBC?
Comment #177
izmeez CreditAttribution: izmeez commentedWow, this is a really complicated issue thread.
I have tried the patch in #174 and found it broke the theme when going back into editing the node.
Possibly there was something else wrong with my configuration at that time.
However, in the process I see this patch removes the submodule media_wysiwyg_view_mode which otherwise seems to be working quite well.
I'm not understanding what the purpose is here and would appreciate any clarification.
Comment #178
Leeteq CreditAttribution: Leeteq commented@izmeez; the main aim (ref. the summary at the top) is to add a way to specifically set the display mode for file attachments that are used within the WYSIWYG area ("body fields"), so that we can have one display mode for the body area, and another/more specific ones (only) for each of the files that are linked to within that same body area. Specifically useful for images and file-type icons within the WYSIWYG area. But yes, this is a complex issue, touching several things.
Comment #179
izmeez CreditAttribution: izmeez commented@Leeteq Thanks for helping me come to grips with this. I have read and tried to understand this thread while exploring configuration of media with wysiwyg embedding. And again reviewed the summary.
So, if I understand where this is at, there is general acceptance of what the submodule media_wysiwyg_view_mode is doing and the proposed next step is an attempt to merge it into the media_wysiwyg submodule.
And also add some further enhancements to it.
I shall try the patch in #174 again in our dev environment and report back.
Comment #180
izmeez CreditAttribution: izmeez commentedThe patch in #174 applies cleanly to the latest dev release and works with existing content.
On the admin file types configuration, admin/structure/file-types/manage/{type}/edit, the Wysiwyg view mode default is none and needs to be set to "Wysiwyg" then everything works when new content is created or edited.
I do not have sufficient confidence with coding to declare the code as reviewed. But, can confirm it is tested.
Comment #181
heddnThe reason for the removal of the view modes module. It isn't needed. It was created previous to the creation of the media_wysiwyg module.
Comment #182
izmeez CreditAttribution: izmeez commentedYes, I understand what the patch is doing and like it.
I see that the patch in #2349977: DoS image derivatives in Media WYSIWYG has some lines overlapping with the patch in #174 and depending on which is committed first the other will need a re-roll.
Comment #183
izmeez CreditAttribution: izmeez commentedNow that #2349977: DoS image derivatives in Media WYSIWYG is committed this patch will need a few lines changed.
Comment #184
azinck CreditAttribution: azinck commentedHere's a shot at a re-roll.
Comment #185
azinck CreditAttribution: azinck commentedOops, failed to rename our update hook. Let's try again.
Comment #187
izmeez CreditAttribution: izmeez commentedThe patch in #185 applies against the new media-7.x-2.x-dev from 2015-02-11 and works as expected.
@azinck Can you please provide an interdiff_174-185
For those who have already been using patch(es) in this thread and have existing data the upgrade will require them to manually set the permissions, Use Media Wysiwyg in editor.
Hopefully, this patch is ready for commit.
Comment #188
azinck CreditAttribution: azinck commentedInterdiffs aren't really possible on rerolls, but here's a rough approximation of one that I've cobbled together to highlight the changes I made:
Comment #189
izmeez CreditAttribution: izmeez commentedThanks, I was wondering the same thing and just finished doing a diff on the two patches to see for myself what had been done to help confirm the changes myself.
Now, we are waiting for someone with enough coding experience to mark this as both reviewed and tested. I can add to the tested part. Although I haven't yet had time to test the additional features support.
Comment #190
izmeez CreditAttribution: izmeez commentedI am going to mark this as RTBC.
This patch is solid in our testing. It applies cleanly. It consolidates the view mode into the media_wysiwyg module which came along later but is where it belongs.
Committing this patch will help move things forward, imho.
If there are any arising issues they can be handled in a new issue. This issue is already very long and has a very useful patch as an outcome.
Comment #191
chiebert CreditAttribution: chiebert commentedI can confirm that it applies cleanly to 7.x-2.0-alpha4+37-dev (2015-Apr-24). I've only tested with image entities, but restriction of view modes in the modal are working, and the default for viewing an embedded image is also taking effect.
Comment #192
Dave ReidI need a bit more time to review and understand the changes going on here (especially the features code), so I'm moving this to a stable release blocker.
Comment #193
partdigital CreditAttribution: partdigital commentedJust applied this patch to 7.x-2.0-beta1 and when I ran updb I go this error:
Error: Cannot redeclare media_wysiwyg_update_7202() (previously declared in /sites/all/modules/contrib/media/modules/media_wysiwyg/media_wysiwyg.install:98)
We need to change the update the hook declaration to media_wysiwyg_update_7203
Comment #194
partdigital CreditAttribution: partdigital commentedComment #195
kevinquillen CreditAttribution: kevinquillen commentedMaybe I am missing something, but I still can't seem to upload an image, insert it more than once into the WYSIWYG using different view modes. The tokens all get overridden with whatever the last view mode chosen in the modal was.
Comment #196
PI_Ron CreditAttribution: PI_Ron commentedThe patch from #185 fails against the latest 7.x-2.x-dev
Comment #199
circuscowboy CreditAttribution: circuscowboy commentedWas successfully using this patch version #143
Media is a fragile ecosystem. I updated my modules to the latest dev and also included media_ckeditor. I have manually applied the patch in 185 and there is a couple of issues.
I have checked off to disable the default, teaser and preview view modes.
They still show up on the list but when they are selected and submitted a js error shows up not allowing them to be submitted.
media_wysiwyg.filter.js?nxzcdc:121
Uncaught TypeError: Cannot read property 'length' of undefined
Others are allowed to be submitted as usual so it mostly works just doesn't hide them any more.
I have attached my patch.
Comment #200
afschFor the view modes issue I got this simple patch that assign the $options variable value to a missed variable.
Comment #201
izmeez CreditAttribution: izmeez commented@alexito it looks like you are suggesting an additional change to one of the patches in the thread. Would you mind adding it to the patch and posting the new patch with an interdiff? Thanks.
Comment #202
Rob C CreditAttribution: Rob C commented@izmeez
An interdiff for a one line patch kinda isn't worth it. Look at the patch. Thanks.
Comment #203
Tom.Camp CreditAttribution: Tom.Camp at CivicActions commentedUpdating path on patch 201 to be run from within the Media module directory.
Comment #204
firewaller CreditAttribution: firewaller commented+1
Any status on this?
Comment #205
joseph.olstadRerolled patch #199 on latest dev
Not sure if this is still the direction we want to go but lets see if the reroll helps move things along.
Comment #207
joseph.olstadComment #208
joseph.olstadComment #210
joseph.olstadcommitted patch #208 to 7.x-2.x dev branch
This will either go into beta8 or perhaps skip beta8 and go straight to 7.x-2.0
Let this bake in 7.x-2.x dev branch for a few days/weeks.
Comment #211
joseph.olstadComment #212
steinmb CreditAttribution: steinmb as a volunteer commented@joseph You have done a great job, moving a lot is stale issues and patches into dev. Thank you! Tough, I would be surprised to to see this not also break stuff. I suggest we roll another beta. It would also allow sites that do not update modules that often to catch up and report back.
Comment #213
joseph.olstadThis is in beta8. I also had to include change suggested by @alexito in comment #200 , without that the simpletest would not pass but with it , it does pass.
Comment #214
steinmb CreditAttribution: steinmb as a volunteer commentedYou move fast :) Looking at the list of blockers https://www.drupal.org/project/issues/search?issue_tags=7.x-2.0%20releas... Is any issues missing from that list or is it still correct?
Comment #215
joseph.olstad@steinmb , I had a look at that list of blockers, I'm thinking it should be trimmed severely, those issues that are not getting any traction stalled out with no workable patch should be postponed and taken off the 2.0 target , backlogged to 3.x or something.
we need the key features that are important stable /stabilized. Otherwise we'll maybe never get past beta.
Comment #216
izmeez CreditAttribution: izmeez commentedWould it help to create an issue tag, Plan for media-7.x-2.1 release, and use that to transfer and reduce the list tagged as blockers for 2.0 release?
Comment #217
joseph.olstadYes that (change blockers from 2.0 to 2.1) could help but I think 3.x would be more realistic because we might want a 2.x that just focuses on current functionality bug fixes. Only well tested 2.0 release blocker patches should make it now, the rest backlog to 3.x
Comment #218
dddbbb CreditAttribution: dddbbb as a volunteer commentedThis conversation has gone a bit off-topic for all subscribed. New issue perhaps?
Comment #219
izmeez CreditAttribution: izmeez commentedYes, indeed. I have opened a new issue #2829038: [meta] Last steps to 2.0 release for that and in the process discovered that the link in comment number #214 is not filtered for just the media project. There are only 4 left that are active or needs work. Kudos to joseph.olstad and everyone else.
Comment #220
joseph.olstadFYI: the changes made above released in beta8 caused regressions which I've attenuated in beta9,
see these issues:
#2828989: Can't edit nodes with images media_view_mode_wysiwyg because missing db tables
#2828949: media_wysiwyg_view_mode ?
This >may< require additional regression fixes. Keeping a heads up on this.
Comment #221
fuzzy76 CreditAttribution: fuzzy76 commentedI have a slightly different regression from this at #2832384: media_vimeo and media_wysiwyg tokenhandling broken except when using the dev version of wysiwyg :-/
Comment #222
joseph.olstadThere was a security vulnerability regression due to this patch which will be fixed in a few moments.
Please upgrade to 7.x-2.0-beta12 immediately if you used this patch or if you have 7.x-2.0-beta8 or 7.x-2.0-beta9 or beta10 or beta11.
Comment #224
mikeskull CreditAttribution: mikeskull commentedWould be nice if this was in the release notes, took me a while to find out what the security issue was. :)
Comment #225
joseph.olstadsorry my internet provider decided to change my IP address just as I was writing this.
Comment #227
rooby CreditAttribution: rooby commentedI opened a follow up issue: #2841742: Media WYSIWYG View Mode configuration lost after media_wysiwyg_update_7205
Comment #228
fox mulder CreditAttribution: fox mulder commentedIs there a chance to implement something similar in D8? Or is this implemented yet?
PS: the problem is entity_embed related in D8. This is a possible solution: https://www.drupal.org/node/2882866