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.
Is there a way we can get this to work when putting images into the body field through a WYSIWYG editor?
Currently using the insert module to put them in body image, which only allows the user to select a single image style. Would it be possible to select the breakpoint group instead?
Comment | File | Size | Author |
---|---|---|---|
#22 | picture-wysiwyg-inline-filter-1885766-22.patch | 24.34 KB | ShaunDychko |
#20 | picture-wysiwyg-inline-filter-1885766-20.patch | 24.58 KB | ShaunDychko |
#19 | i1885766-19.png | 4.77 KB | attiks |
#18 | picture-wysiwyg-inline-filter-1885766-18.patch | 23.99 KB | ShaunDychko |
#15 | picture-wysiwyg-inline-filter-1885766-15.patch | 23.6 KB | ShaunDychko |
Comments
Comment #1
attiks CreditAttribution: attiks commentedIt is definitely possible to extend the insert module to handle this, but
If you have a patch feel free to upload it, so we can discuss the best way to handle this. If you want us to work on this, feel free to contact me using my contact form
Comment #2
attiks CreditAttribution: attiks commentedFor anybody wanting to implement this: "The proper way would be to insert a custom token instead of the full HTML", Jelle_S started added support using the media module, but in edit mode you'll see the fallback image. The output uses the 'picture' tag.
The problem is related to how CKEditor handles empty span tags.
If you want to try this, install latest versions of picture, media and insert a picture into the body of a node.
Comment #3
ShaunDychko CreditAttribution: ShaunDychko commentedWhat do you think of this?
I'm approaching from an IMCE, WYSIWYG, CKEditor angle, but this might work for the Media module also:
Configure CKEditor to put a custom data attribute in the img tag with the picture group machine name, such as:
<img src=... data-picture-group="[picture_group_machine_name]">
CKEditor would have a select box for choosing the picture group in the CKEditor image dialog. Then create an input filter searching the post for img tags with the data-picture-group attribute set. This input filter would create an array with keys similar to the Render Array that's passed into
picture_file_formatter_picture_view
. The input filter would have a function that duplicates most ofpicture_file_formatter_picture_view
, just omitting the parts specifically for file fields. This is where things get murky for me, but I'm hoping that, one thing leads to another, and the needed markup gets returned to the input filter and can be used to replace the original img tag.I can take care of the CKEditor part of this, but I could use some tips for the rest.
I suppose another issue would be to copy over other attributes of the original img, but maybe that should be another issue.
Comment #4
attiks CreditAttribution: attiks commentedMedia module already has WYSIWYG support, but for stand alone use, your approach looks good. Maybe it would be good to add a data attribute for the fallback image as well.
Using picture_file_formatter_picture_view is indeed the way to go. If you have any troubles with it, let me know.
What do you mean with "I suppose another issue would be to copy over other attributes of the original img, but maybe that should be another issue.", what attributes do you want to copy?
Comment #5
ShaunDychko CreditAttribution: ShaunDychko commentedPerhaps
picture_field_formatter_view($entity_type, $entity, $field, $instance, $langcode, $items, $display)
is a slightly better place to look for inspiration? It doesn't have any file_entity code in there...Comment #6
ShaunDychko CreditAttribution: ShaunDychko commentedHere's a first attempt at creating an inline filter. Hopefully it's easier for you guys to review this than do it yourself!
There are still several TODO's but I'm just checking in to see if things are on the right track.
One issue is that images inserted by IMCE have a root relative path, and I'm not sure how to turn this into a 'URI' with a 'Schema', as expected by theme_picture. The documentation for theme_picture suggest that the 'uri' it expects can be relative to 'base_path()', but this isn't the case, but please correct me if I'm wrong. The image_style_url() function in theme_picture turns '/sites/default/file/filename' into 'http://sites/default/files/styles/[style-name]/public/sites/default/file...', which returns an error. Currently, the filter assumes the inserted image must be public, and removes the 'file_public_path' portion of the root relative path and prefixes it with 'public://'.
Other TODO's include adding an admin section to configure the fallback image per picture group, and also make a nice interface in the CKEditor image dialog to insert the picture_group machine name as the data-picture-group attribute.
Looking forward to your feedback.
Comment #7
ShaunDychko CreditAttribution: ShaunDychko commentedOh, and to answer your question about other attributes: The class attribute of the img tag would need copying, if this is the method a site builder is using to float images, or otherwise the inline style attribute would need copying, I suppose to the root span tag that's returned by the picture module. I don't use inline styles, but definitely copying classes would be important.
Comment #8
ShaunDychko CreditAttribution: ShaunDychko commentedHere's today's progress. There are a lot of additions, but it's shaping up pretty well. The only thing left to make is a CKEditor plugin that will modify the CKEditor image dialog by adding a select list for picture groups. There's a settings page added with this patch that configures which picture group to show in CKEditor, with a fallback image style per group, and even a weight to control sort order. Hopefully on Monday I'll have the finished integration! Currently everything works with hand coded image tags. I also added extra data attributes to the top level span tag so that styles can be applied more easily. For example, the following code entered in the WYSIWYG
will result in
and on Monday I'll make CKEditor generate the image tag markup in a user friendly way. Using custom data attributes in this way is much more convenient with CKEditor than using classes.
Comment #9
sagannotcarl CreditAttribution: sagannotcarl commentedShaun Dychko, this is great! I've applied your patch and everything looks good so far.
You may have already come across this but there is a small typo in your patch with a permission name. The access callback for $items['admin/config/media/picture/ckeditor'] should be "administer pictures" not "administer picture".
I'm really excited to see what you come up with for the rest of it! Let me know what I can do to help.
Colin
Comment #10
attiks CreditAttribution: attiks commentedAmazing job, kudos!
General remarks:
this can stay on one line.
the quotes aren't necessary
no need to translate numbers, can be written as drupal_map_assoc(array(1, 2, 3, ...)
is there a possibility that there will be side effects?
probably better to use http://api.drupal.org/api/drupal/modules%21system%21system.api.php/funct...
But is it really necessary to do this?
Comment #11
attiks CreditAttribution: attiks commentedMaybe it is easier/better to use regular expressions to extract the pictures, see attached patch
PS: Pull first, I changed the line endings in picture.admin.inc
Comment #12
attiks CreditAttribution: attiks commentedAFAIK there's no drupal function for this, so two options:
Insert the uri as a data- attribute when the file is inserted (will only work for managed files)
Loop all stream wrappers till we find a match, see file_get_stream_wrappers(STREAM_WRAPPERS_WRITE_VISIBLE) to get all wrappers and getDirectoryPath() to get the real path
Comment #13
sagannotcarl CreditAttribution: sagannotcarl commentedHey Shaun Dychko,
Any luck on the interface aspect of this? I may need to work something like this for a project this week and I'd love to be building towards the same thing.
Colin
Comment #14
ShaunDychko CreditAttribution: ShaunDychko commentedHey Colin, the big reveal's lined up for tomorrow! Spent today wrestling with the WYSIWYG module to figure out the best way to add a CKEditor plugin that I already built for resp_img, and learning how to send PHP variables to JS. More details tomorrow. Thanks for the interest! I'll be looking forward to your feedback and code review. I'll feel much better using this on client sites after it's passed scrutiny from other developers (who have more experience than myself!).
Comment #15
ShaunDychko CreditAttribution: ShaunDychko commentedOK ladies and gentlemen, here's the first draft of the whole package.
@Carl, thanks for noticing the access callback typo, and it's fixed.
@attiks, thanks for all the suggestions, and I've implemented them all. In #11 you mention "Pull first, I changed the line endings in picture.admin.inc", but I don't see anything new, so this patch is against http://drupalcode.org/project/picture.git/commit/10d1ffb
With finding the Drupal URI from the root relative image src, there was a nice side effect that imce_wysiwyg now works with private images. It didn't before since it inserted src paths like /sites/default/files/private/image.jpg which are forbidden by .htaccess. Bonus! (I guess... but who uses private images anyway? They're not even really private unless managed by an access module...)
Getting the CKEditor plugin loaded by the WYSIWYG module was a real puzzle. There seems to be an issue with loading plugins that don't have buttons, and the only other Drupal module I could find that has a no-button WYSIWYG plugin is imce_wysiwyg, so the technique here borrows from that module. The CKEditor plugin will be loaded anytime a picture group has been enabled at /admin/config/media/picture/ckeditor. Loading the plugin is not controlled by a checkbox on the WYSIWYG button/plugin config page. More details are in the code comments.
When testing this, you'll probably want the picture_wysiwyg.css file in your admin theme. Please have a look at the comments in the file for details.
@attiks re: picture_page_build. I changed it from picuture_page_alter. Yes, the JS has to be loaded somehow, and the technique used with the fields doesn't work for some reason. I couldn't find good documentation for the #attached property, and the field hooks aren't even triggered for the node body field, so I guess this is treating differently? Maybe drupal_render is needed? I have no idea. But in any case, if the picture module is being used, it's probably needed on most pages, in which case there's a performance advantage to having it aggregated, which is possible only if it's included on every page. The lines 1056 to 1060 in _picture_filter_prepare_image()
should probably be removed since I don't think they do anything.
I've tried to include as many code comments as I could, so I let those tell the rest of the story. I'm looking forward to your feedback!
Known issues:
Comment #16
attiks CreditAttribution: attiks commentedNice job, it works. For anybody wanting to test you'll need the following
You need to patch wysiwyg, see #1853550-97: Ckeditor 4.0 - The version of CKEditor could not be detected., in short replace
\'
twice with[\'"]
on the line starting with if(preg_match('@version:
You need to enable your picture groups for integration at
admin/config/media/picture/ckeditor
Comment #17
attiks CreditAttribution: attiks commentedthis can just use 'enabled' instead of $machine_name . '_enabled', same for weight and fallback
is this still needed?
trailing space
dot at the end
can we change the labels on the insert screen as well?
Comment #18
ShaunDychko CreditAttribution: ShaunDychko commentedThanks attiks for the feedback. Here's the latest.
I ran this through the Coder module on the "minor" settings to find any other issues in this patch.
I'm not exactly sure what you mean by "can we change the labels on the insert screen as well?" Do you mean the CKEditor image dialog? On the CKEditor image dialog the displayed labels are the human readable breakpoint groups. We can add a variable from the settings page to set the label for the 'not_set' value on the CKEditor image dialog, if that's what you mean. Anyhow, your comment inspired me to update the settings page at /admin/config/media/picture/ckeditor with Breakpoint Group human readable names, rather than machine names. I've also added a CSS selector suggestion since site builders wouldn't otherwise know the machine name to target. I also updated picture_wysiwyg.css which has example CSS for styling these images, and added an @file block to picture_ckeditor.js.
Thanks for mentioning testing instructions for the patch. I should have done that. I would write it a little differently, although I'm sure your instructions work:
Apply the patch #73 to wysiwyg: http://drupal.org/node/1853550#comment-6968584 if you're using CKEditor 4.x. This patch for Picture was developed with CKEditor 4.0.1, but it should probably work with 3.x since all it does is modify a dialog (nothing API heavy). There are some comments in http://drupal.org/node/1853550 that the CKEditor standard download is missing buttons, so the link I've put above is to the "Full" version of CKEditor (a distinction that I think is new with v. 4)
There's no need to
since this patch doesn't support that module. Doing so would not be too hard, probably, since one would search the ckeditor module for a hook that fires when the CKEditor is being prepared for the page, and then load the JS as shown in picture_wysiwyg_plugin(). I don't use the ckeditor module, which is why I went with the wysiwyg module.
Include picture_wysiwyg.css in your theme CSS in order for this to be included in the CKEditor edit area, assuming you have set the WYSIWYG profile to use theme CSS. If you've set "Define CSS" at admin/config/content/wysiwyg then manually include the picture_wysiwyg.css file. This is needed for having images align properly in the edit area, and you might also consider styling the image size, perhaps using the example commented out in picture_wysiwyg.css for inspiration and substituting your own data-picture-group values.
Then, yep, enable picture groups for CKEditor integration at
admin/config/media/picture/ckeditor
as you said.Let me know what's next ;)
Comment #19
attiks CreditAttribution: attiks commentedI mean this label (in the dialog)
Comment #20
ShaunDychko CreditAttribution: ShaunDychko commentedDone.
Comment #21
attiks CreditAttribution: attiks commentedSome minor things, but I'm probably going to commit this during the weekend, thanks!
I assume this can go?
dot needed at the end
add a @see picture_field_formatter_picture_view()
add a check to make sure we have a src
this -> This
not needed anymore?
no space after ( or before )
debug leftovers?
Comment #22
ShaunDychko CreditAttribution: ShaunDychko commentedWith a commit on the horizon, this is getting exciting! Yes, definitely, let's remove the filter cache => FALSE.
I've also looked carefully at all the comments and made sure they're formatted as // [Capital] ... period.
Added the src check, reformatted the bracket spacing in the JS file, and removed the comments regarding "this", which were just to indicate what the JS object "this" referred to in that context.
I also added:
to make the alert message refer to the proper label on the imageSize box.
And I added:
in hook_uninstall to clean things up.
I also removed:
from line 1056 of picture.module since that isn't actually doing anything, and the '#attached' property isn't used in either theme_picture_formatter() nor theme_picture().
Thanks a lot for your feedback!
Comment #23
attiks CreditAttribution: attiks commentedCommitted, thanks Shaun
Comment #24
dddbbb CreditAttribution: dddbbb commentedDoes the project description need to be updated to reflect this change? Under the sub heading "Difference with Responsive images and styles" it currently reads "no support for inline images (feel free to open an issue)".
Comment #25
attiks CreditAttribution: attiks commented#24 project info updated
Comment #26
ShaunDychko CreditAttribution: ShaunDychko commentedThanks a lot attiks for committing this, and for the code reviews. I'm looking forward to using this module!
Comment #27
akmalfikri CreditAttribution: akmalfikri commentedSubscribe.
BTW is this also including the WYSIWYG Media module?
Comment #28
ShaunDychko CreditAttribution: ShaunDychko commentedIt should work with Media if the Picture input filter is enabled after the media input filter in your text format configuration. You'd have to double click on images to insert the data-picture-group attribute after inserting it with Media, and you'd also want to insert a high resolution picture (preferrably the original file).
Comment #29
flocondetoileHello,
Really awesome and promising module.
I test this feature with media module 2.x (latest dev version) and wysiwyg module
I create 2 display mode of file in media (file display)
1- the first, called, "large" use a display of core's image (with a style fixed)
2- the second, called "responsive" use a display of picture (with a picture group)
I inserted an image with media module in a body field and used the both display mode created
1- In the first case, the same style (large) is always used. I double click on image to insert the data-picture-group attribute after inserting it with Media, and save. But the configuration of the data-picture-group attribute is not saved
2- In the second case, It's the same thing, the data-picture-group attribute is not saved when I double click on image to insert them. But the image is responsive, with the display mode i called "responsive" wich is defined on a picture group (in /file-types/manage/image/file-display). You can see in the html this span with data-attribute empty.
In both case, the attribute image-alignment is not saved too.
Otherwise, are you plan to include CKEditor module too ?
Thanks
EDIT: it's done : media module support
Comment #30
ShaunDychko CreditAttribution: ShaunDychko commentedTo avoid overload on this issue, it sounds like there are two separate issues here: 1) CKEditor module. Definitely doable, but not there yet. 2) Supporting the Media module in the WYSIWYG. Please create these issues with your notes, and I'll try to help there when I can if no one else gets there first.
The current implementation works as per #18 with IMCE.
Comment #31
invantix CreditAttribution: invantix commentedI want to understand what I need to do to use picture with wysiwyg.
Do I need to use the latest dev version of Picture?
Do I need to also install one the patches above?
How did I install the patch?
Thanks!
Comment #32
attiks CreditAttribution: attiks commented#31 you need the latest dev version of picture, the patch is already included.
Comment #33
retrodans CreditAttribution: retrodans commentedSo, upon looking in the new dev module I can see the patch has indeed been applied (although with a few tweaks). yet sadly it doesn't appear to be working for me. So my question is, does it work for everyone else, and if so, was there any other dev/patched versions required of other modules, is the end of this ticket implies this is no longer necessary, but wonder if anything has changed since.
Thanks,
Dan
Comment #34
retrodans CreditAttribution: retrodans commentedIgnore that, schoolboy mistake i'm afraid. If anyone else gets to this point, to enable this it isn't automatic, you need to go into your WYSIWYG editor and check the responsive checkbox as you would things like bold/italic/imce.
Comment #35
attiks CreditAttribution: attiks commentedIf anybody feels like writing documentation (on d.o. or a blog post) that would be great, I don't have much time for the moment
Comment #36
flocondetoileDocumentation available http://drupal.org/node/1983312
Feel free to review it. English is not my native language.
Comment #37
attiks CreditAttribution: attiks commented#36 Thank you!
Comment #38
fbrachere CreditAttribution: fbrachere commentedHi,
there is a problem if drupal is not installed at root ( / ).
I've a site which is reachable at http://www.example.com/drupal7/ and the module doesn't translate
img
intospan
in CKeditor (but it works fine with image in a node).The problems seems to come from
function picture_image_uri($src)
(picture.module file).If I put
$needles[$scheme] = "drupal7/" . $needles[$scheme];
after the line
$needles[$scheme] = trim($stream_wrapper->getDirectoryPath(), '/');
everything is working fine.
This is an ugly hack but I'm a newbie to Drupal and don't know how to fix this in an elegant manner.
Comment #39
attiks CreditAttribution: attiks commentedCan you try it with
$base_url . '/'
? make sure to addglobal $base_url;
on the line above.Comment #40
fbrachere CreditAttribution: fbrachere commentedNo, it doesn't work.
I did this:
And the result of
$needles[$scheme]
is:http://www.example.com/drupal7/sites/default/files
With previous changes, it was:
drupal7/sites/default/files
Comment #41
attiks CreditAttribution: attiks commentedmy bad, try with $base_path
Comment #42
fbrachere CreditAttribution: fbrachere commentedAlmost...
Works with:
Because of the leading '/'
Thank you for your help.
Comment #43
attiks CreditAttribution: attiks commentedComment #45
barber75 CreditAttribution: barber75 commentedhey guys,
I have the latest dev version and have configured Picture and WYSIWYG CKeditor to use the responsive image settings from picture.
When I select my photo in CKeditor on my content type, I can add the picture style to it, and the data tag gets added to the img tag. However, the responsive images aren't being injected onto the page...
Is there anything I'm missing here?
Thanks!
Craig
Comment #46
attiks CreditAttribution: attiks commented#45 This issue is closed, so can you open a new one in the future, otherwise we might miss it.
To answer your question: did you enable the picture option in your text format?