Previously in D6, we were able to simply remove the JS for the teaser splitter, which injected a new textarea solely via JavaScript. Ill concept.
In D7, a text field with a summary actually consists of two separate form elements, which also end up being stored in separate database field columns.
This code no longer works:
function wysiwyg_form_alter(&$form, &$form_state) {
// Teaser splitter is unconditionally removed and NOT supported.
if (isset($form['body_field'])) {
unset($form['body_field']['teaser_js']);
}
}
If we continue to simply remove it, then that would mean that the "summary" column of text fields would _always_ be empty when using Wysiwyg.
However, since the summary is properly stored now, there are use-cases for displaying the summary of a text field only. For example, Views may just retrieve the summary for lists and overview pages.
Quick fix: Remove the JS and hide the summary field.
Long-term proper fix: Allow to attach an editor to multiple fields at the same time.
Comment | File | Size | Author |
---|---|---|---|
#61 | wysiwyg-summary.741606.60.patch | 23.62 KB | TwoD |
#12 | 741606-summary.patch | 6.31 KB | muriqui |
Comments
Comment #1
JacobSingh CreditAttribution: JacobSingh commented@sun
I think the quickfix is not a great solution because even as a textarea it is better than nothing until we fix it properly.
Just so I'm sure I undertand what you want to do, the work here is to allow the summary field to also be rendered as a WYSIWYG?
Best,
J
Comment #2
sunTo fix this, we need to
Comment #3
wilgrace CreditAttribution: wilgrace commentedsubscribe - thanks for the work on this...
Comment #4
bryancasler CreditAttribution: bryancasler commentedsubscribe
Comment #5
Gogowitsch CreditAttribution: Gogowitsch commentedsubscribe
Comment #6
pillarsdotnet CreditAttribution: pillarsdotnet commentedReferenced in #221257: text_summary() should output valid HTML and Unicode text, and not count markup characters as part of the text length See comment #93: Summary requested by ysched.
Comment #7
zilverdistel CreditAttribution: zilverdistel commentedWhat is the status on this issue? Does the patch in http://drupal.org/node/221257#comment-4475410 solve this issue? That means: does it offer a wysiwyg editor on the teaser field in drupal 7?
--EDIT--
I tested that patch, and it does not solve this issue.
Comment #8
pillarsdotnet CreditAttribution: pillarsdotnet commented@zilverdistel: The status is that sun has identified a problem and suggested a solution strategy, but nobody has written a patch. You can help either by writing a patch, or by asking another programmer to write a patch.
Comment #9
zilverdistel CreditAttribution: zilverdistel commented@pillarsdotnet: I think there are other ways of helping than the ones you describe (e.g. bug reporting, test reporting, ...). I hope I'm not misunderstood, I'm thankfull for all the efforts you put in the drupal community/code!
Comment #10
mstrelan CreditAttribution: mstrelan commentedsubscribe (http://3281d.com/2009/03/27/death-to-subscribe-comments)
Comment #11
BeatnikDude CreditAttribution: BeatnikDude commentedsub'n
Comment #12
muriqui CreditAttribution: muriqui commentedI think this should just about do it.
There's a minor bug with this approach, but I'm not sure it's worth holding up this feature over. To reproduce (in Google Chrome, at least):
The grippie being out of place doesn't seem to have any negative effect other than aesthetics... It's non-functional while the rich-text editor is enabled for that field (i.e. the rich-text editor's resize grippie still wins), and it pops back in place if you again disable rich text on the Body. Not sure where to look to fix this, but if the patch is otherwise acceptable, I could open up a separate issue for the grippie problem.
Comment #13
mstrelan CreditAttribution: mstrelan commentedJust tested this out on a basic page with TinyMCE editor on the body field, using Full HTML. The node already existed with body content but no summary.
I might add that I am on Wysiwyg 7.x-2.1 rather than dev so had to apply the patch manually. Perhaps someone can confirm if these steps produce the same errors on the latest dev snapshot?
Comment #14
muriqui CreditAttribution: muriqui commentedHey mstrelan, I just setup a new test environment with TinyMCE and ran your test, but with a patched Wysiwyg 7.x-2.x-dev instead of 7.x-2.1, and I couldn't reproduce your results. Everything worked fine for me; no JavaScript error, and TinyMCE reloaded properly. Can you retry with dev and see if the patch works cleanly? Thanks!
Edit: Also be sure to clear your caches after applying the patch.
Comment #15
muriqui CreditAttribution: muriqui commentedUpdated the patch to fix a bug that occurred when the "Summary input" option was disabled on the field.
Comment #16
mstrelan CreditAttribution: mstrelan commentedOk using latest dev snapshot solved it, sorry I should have done that originally. I tried this time with the patch from #15 and it works (except the problem with grippie).
Comment #17
sterndata CreditAttribution: sterndata commentedsub
Comment #18
dawnbuie CreditAttribution: dawnbuie commentedHi muriqui this patch works! It's great - exactly what I needed.
Will it be incorporated into WYSIWYG or will I have to repatch for each update?
thanks again.
Comment #19
muriqui CreditAttribution: muriqui commentedHi dawnbuie. Whether or not the patch gets committed to WYSIWYG isn't my call; I'm not a maintainer on the module. But if you've tested the patch and found it to be a good solution, feel free to change this issue's status to "reviewed and tested by the community" to indicate to the maintainers that it's ready for inclusion. Thanks!
Comment #20
hanoiisub
Comment #21
plachRerolled #15 and tested it: everything seems to perform well, even the grippie, on Firefox 10, Chrome 17, Safari 5.1.1, Opera 11.51, Internet Explorer 7-8-9.
Tentatively marking RTBC to get some feedback from the maintainers.
Comment #22
plachI just had an error in
wysiwyg.js
while uploading an image in an image field. Reverting the patch fixes the problem. I'll try to provide a new patch fixing the issue.Comment #23
plachWe were missing a multiple detach:
Comment #24
plachComment #25
peximo CreditAttribution: peximo commentedWorks as described.
Comment #26
robhoward79 CreditAttribution: robhoward79 commentedCan confirm the patch at #23 works great :D
Comment #27
muriqui CreditAttribution: muriqui commented+1 for the patch in #23... Just retested and confirmed that the splitter works and that the issue in #22 has been resolved. Thanks, plach.
Comment #28
Bernsch CreditAttribution: Bernsch commented+1 for the patch in #23... Just retested and confirmed that the splitter works! Thanks!
Comment #29
ronline CreditAttribution: ronline commentedPatch from comment #23 tested on 7.x-2.1+13-dev version.
Spliter works fine.
Comment #30
Alan D. CreditAttribution: Alan D. commented@muriqui & plach You rock :)
Initial tests on Drupal 7.12 + WYSIWYG 7.x-2.1+13-dev + TinyMCE are running perfectly and have resolved the 5 year headache of how to handle the teaser correctly.
Please consider rolling this into the latest dev for widespread testing.
Comment #31
TwoDInteresting!
This does actually work. This is a pretty big decision to make so I'd like Sun's opinion as well, but it's a go from me.
Comment #32
sunThanks for the all the work on this!
This is a tough call to make. I'll explain in detail:
This is quite an API change. I wonder whether contributed and custom modules will break.
The only code that wouldn't be affected are overridden the editor callbacks on the client-side, which rely on the passed in settings only.
When we did the conversion to
triggers
, IIRC we also had the idea in mind to convert the trigger/field definition into a jQuery selector, which doesn't expect HTML IDs by default (like now). (or respectively, see #2)In turn, you'd able to specify a
trigger.selector
of'#field1, #field2'
, or.sharedFieldClass
, or even#field-foo .summary, #field-foo .value
.Of course, that would mean an API change, too; likely even a larger one, since editor callbacks and other code currently expect params.field to be a single #ID. Also, target elements don't necessarily have a HTML ID yet, so it possibly has to be generated during the attach process first.
At this point, I'm not sure what's the better solution yet.
The multiple selector approach would possibly have the downside of not allowing separate settings per matched field (e.g., to use a simplified toolbar on the summary vs. full body).
The multiple fields per trigger approach has the downside of duplicating a lot of Drupal.settings in the output.
Lastly, I'm not able to see any conflict, but I wonder whether anything in our plans for 3.x might help with deciding on either approach?
In any case, it looks like this API change should go into 3.x only (which in turn might shift our 3.x roadmap into 4.x [or not]). That is, unless we have sufficient confirmations that people are using other contributed modules that integrate with Wysiwyg and they don't break due to this change. I don't see any reports regarding that in this issue yet.
Minor: At least this would have to check
isset('summary')
first, so the 'summary' key is not created when it doesn't exist.Comment #33
erbzh CreditAttribution: erbzh commentedPatch from comment #23 works great (Drupal 7.14 + Wysiwyg-7.x-2.x-dev 2012-04-30).
Thanks !
Any plans to roll this patch into a coming dev version of wysiwyg ?
Comment #34
mesr01 CreditAttribution: mesr01 commented@plach: Your patch really made my day. A big thumb up to you!
Comment #35
Anonymous (not verified) CreditAttribution: Anonymous commentedThanks for the patch in #23!
Any chance to get an updated version for the latest dev? Or maybe even commit it?
Comment #36
Alan D. CreditAttribution: Alan D. commentedLooks like the logic here changed with #1388224: editors detach on AJAX form submission, pushed (July 1, 2012)
The previous snapshot before this change was committed is 2012-06-26 and patch #23 still cleanly applies. You can access this here or via http://drupalcode.org/project/wysiwyg.git/shortlog/refs/heads/7.x-2.x and look at the dates to find this commit.
Comment #37
smk-ka CreditAttribution: smk-ka commentedReroll of #23, incorporating the changes from #1388224: editors detach on AJAX form submission, but nothing else.
Comment #38
plachWhat about committing the patch onto the 7.x-2.x branch right after the next stable release and try to have some wider testing to see if anything breaks? If it turns out that nothing breaks we would have an approach to experiment with and we could decide in 3.x whether keep it or switch to the other.
Comment #39
krima CreditAttribution: krima commentedThe patch at #37 works perfectly! Thank you very much.
Comment #40
Bernsch CreditAttribution: Bernsch commentedCommitted to 7.x-2.x. ???
Succeeds the patch in #37 or #23 in the next official release?
Comment #41
Bernsch CreditAttribution: Bernsch commentedThe patch at #37 works for me too! Thank you very much!
Comment #42
halefxI can also confirm that the #37 patch works with the 2012-06-26 dev version. Thanks!
Comment #43
wickwood CreditAttribution: wickwood commentedPatch #37 also works for me, however I could not get it to apply to the 2012-06-26 dev version. I did get to apply 2012-07-21 dev version. I used this version because I believe it was the active head when Patch #37 was created and it should have been the version the patch was made from.
Thanks for contributing this, I've been frustrated by this for a long time and hope the patch gets committed soon. I think it will save many people from the confusion created when the exposed summary field does not have a WYSIWYG editor.
Steve
Comment #44
Bernsch CreditAttribution: Bernsch commentedCan we commited Patch #37 to dev version or in the next Recommended releas 2.3 ?!?
I think it's time, because the creation date of this issues: March 13, 2010! ;-)
Thanks!
Comment #45
TwoDHere's an alternative patch which aims to minimize the API changes (as well as the amount of settings duplication).
It does this using a new
summary
property in the trigger object which simply holds the id of the summary element. Any time an editor is attached or detached, this property gets checked and any actions are duplicated for a separate summary editor instance.One significant difference compared to the #37 patch is that the instances are always toggled together, with one "Disable/Enable rich-text"-link.
I don't think this should break any contrib modules, at least not if they are using
wysiwygAttach
andwysiwygDetach()
to manipulate editor instances.Comment #46
tripper54 CreditAttribution: tripper54 commentedMedia module embed codes break on edit in the summary when using patch #45 & CKEditor.
Steps to reproduce:
1) create a new node
2) use the wysiwyg media embed button to add an image to the summary.
3) save the node. All works well
4) edit the node
The wysiwyg editor HTML encodes the media embed tag, so if you disable rich text, instead of seeing
[[{"type":"media","view_mode":"media_preview","fid":"700","attributes":{"alt":"","class":"media-image","height":"180","typeof":"foaf:Image","width":"180"}}]]
you see
[[{"type":"media","view_mode":"media_preview","fid":"700","attributes":{"alt":"","class":"media-image","height":"180","typeof":"foaf:Image","width":"180"}}]]
I'm not sure if this is a problem with the media module or this patch. It works fine in the body field though, it's only the summary that tries to encode the embed tag.
Comment #47
tripper54 CreditAttribution: tripper54 commentedRe #46, some changes to the media module were required, see http://drupal.org/node/1995030 . So all is good here.
Comment #48
Pene CreditAttribution: Pene commented#45 works well. thank you TwoD!
Comment #49
NaX CreditAttribution: NaX commented#45 works great. Anything stopping it from being committed.
Comment #50
omerida CreditAttribution: omerida commented+1 for #45 here too, tested and works for me.
Comment #51
watchdog CreditAttribution: watchdog commented#45 thanks, works great!
Comment #52
dawnbuie CreditAttribution: dawnbuie commented#45 thanks - why not commit?
Comment #53
bskibinski#45 Awesome work! Works on drupal 7.24
Comment #54
NaX CreditAttribution: NaX commentedSince this is an incompatibility with a core feature I think this should be bump up to a major bug, not a feature request.
Comment #55
TwoDI discovered a couple of annoying issues with the previous patch.
Editors did not always like being attached to an invisible field (summary) and would behave oddly (wrong widths, not fully initialized) until disabled and re-enabled for that field.
Whether an editor was enabled or not would be lost during AJAX operations such as "Add another field".
To compensate for this, I delayed the creation of the summary field instance until the field is shown, and added an internal variable to keep track of which editors have been disabled.
This also allowed me to simplify some logic in wysiwyg.js, though it is still a bit convoluted.
Comment #56
drupov CreditAttribution: drupov commentedThe patch from #55 works great. My environment: drupal 7.26, wysiwyg-7.x-2.2+29-dev
Comment #57
dariogcode CreditAttribution: dariogcode commentedI applied #55 in 7.x-2.2+26-dev and works well.
Comment #58
brockfanning CreditAttribution: brockfanning commentedIt looks like line 139 of wysiwyg.js has changed, so the patch doesn't apply to the latest dev. I edited the patch to see if it would apply (and it did) and the patch still seems to work, but I'm not sure if the recent change to line 139 needs to be incorporated into the patch.
Comment #59
cimo75 CreditAttribution: cimo75 commentedHi can you please help me to apply the patch to the last dev release?
tx
Simone
Comment #61
TwoDI still wasn't happy with the way settings and states had to be passed around in the #55 patch, so I put some more time into this and built another patch which I've also tested with some other patches coming up.
It's not far from the #55, which worked very well for folks, and I needed to get this done fast so I've committed the below patch to 7.x-2.x.
Thanks to everyone involved for patching, reviewing, testing, and commenting!
We can finally close this for D7!
@cimo75, you don't need to do that now, just download the 7.x-2.x-dev snapshot (will be regenerated within 12hrs).
(I'll likely backport some parts of this patch for D6, but I can't guarantee the actual summary functionality will be possible there.)
Comment #62
Alan D. CreditAttribution: Alan D. commentedThank you so much!
Comment #64
caw67 CreditAttribution: caw67 commentedgenerally works! but cant edit filefields on the same node.
error: An error occurred while attempting to process /file/ajax/field_bilder/und/form-WgFZwbsvB6T00xqkswTn0RKw6Sv8IjUBG-OizJYWeik: e is undefined
Comment #65
Alan D. CreditAttribution: Alan D. commented@caw67
It would be best to open a new issue and provide as many details as possible about the issue. For example, versions of Drupal, core File field or Media (or any other modules that you are using for the file upload), if you are running JQuery Update and what versions and finally the editor (CK, etc).
We have been using this or one of the version of the patch for years without issue.