Posted by greg.harvey on July 22, 2009 at 4:34pm
| Project: | Mark-up Snippets |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
I've looked in to this and tried various techniques suggested in the WYSIWYG issue queue, but thus far I've been unable to get anything working. If someone else could try I'd be truly grateful. Related issue is #356480: Lazy-load editors
Comments
#1
I've created a patch to make this module aware of Wysiwyg module, if it's enabled. The JS integration file will take care of detaching any editor before the AHAH call and re-attach it afterwards. Note that the added code does not do everything Wysiwyg module does when detaching and attaching editors, but just enough to deal with Markup Snippets' AHAH request.
I also took the liberty to rewrite the form_alter implementation and the AHAH callback so the hook takes care of the form changes and the callback only does rendering. An extra submit handler was needed in that case. This way the cached version of the form should always match what the users are seeing, as is recommended.
The first attempt I made at this required me to move the markup_snippet fieldset out of body_field, but even tho that was not required later, it worked nicely with Node form columns, so I left it in there. The default weight should still keep it just above the body textarea.
The new way this is done will probably also make it easier to fix #521400: Selecting a snippet breaks teaser splitter - need to rebuild body form field using node.module function.
#2
Wonderful! I will test this next week. I happen to be working on the project we built it for anyway next week, so your timing is perfect. Thank you so much. This is a great patch, much needed. =)
#3
I just noticed I forgot to deal with deactivated editors (they will reactivate after the snippet has been inserted)-, will post an updated patch tomorrow, 3am here...
#4
Here's an improved patch. It simplifies the logic a bit and can also handle disabled editors.
My editor also decided to remove the whitespaces on empty lines...
#5
#6
Hi,
There were some other changes not yet committed to dev. I've committed them now. Your patch wouldn't apply, I'm afraid, so could you please re-roll against the latest dev snapshot? (It might take a few hours to appear.)
Also, could you use this diff command, as it makes the patch easier to read:
diff -up original.php new.php > filename.patch(See: http://drupal.org/node/323)Thanks again! =)
#7
Ok, I'll do that.
That is exactly what I did, except that I also used the -r and -N switches to make it recursive and catch the added .js file (couldn't diff against CVS as I don't have permissions to
cvs addit). The commands used are listed in the patch.EDIT: Oops, I just noticed that a backup file created by my editor was included in the patch (markup_snippets.js~). My bad...
#8
Thanks, sorry to mess about with the dev snapshot. I forgot there were uncommitted changes. I think the snapshot will update in about 3 hrs time.
#9
No problem, I checked out the DRUPAL-6--1 branch from CVS instead. Btw, I noticed you added a "Use snippet" button but it's commented out. Do you want to keep the replace/append-on-select functionality, or do you want to replace it with a button to initiate the AHA call? Just wondering since I thought I might as well use that code to hook in the submit handler from my patch instead of adding yet another unused button for that. But I want to know if I should make it
#access => FALSEso it doesn't show up, or if I should move the#ahahfrom the select box to the button?#10
Oh, I forgot I did that. If you think the on select behaviour is robust enough, keep that. I guess I was experimenting with the button, but if it's commented don't worry about it - in fact, feel free to delete completely. =)
#11
Hey, how you getting on with this? Anything I can do to help? If you can send me code, I don't mind rolling the patch for you. =)
#12
I'm having some problems with it but I've got a debugger up and runnning now so I can step through the callback, form builders, handlers and anything else involved. The custom submit handler is not getting called, so the default (node_form_submit) is called instead and the node is saved each time a snippet is inserted.
I changed the JavaScript implementation a bit so it re-serializes the form after detaching the editor, or the body POST data would not contain the latest changes, which is needed if the append method would ever work as expected.
#13
Sorry to hear it's giving you pain. Keep us posted. Appreciate you putting the time in.
Would you like to be a co-maintain, btw? If you have CVS access, I don't mind adding you.
#14
Hey,
Want to post your code so far? Perhaps me or somebody else can take a look? =)
#15
Ps - I'm half tempted to roll back to the version of the dev snapshot you had a working patch for. It seems to me that having proper WYSIWYG support is a bigger win than anything I may have added in the intervening time. I don't think I did anything significant. What do you think? Does your patch in #4 work, to your knowledge?
#16
I'm sorry for the very late response. The code in #4 was unreliable and did not work as I expected. I have a version working well now, will create a patch and post it asap.
It currently uses the "Use snippet" button as it was much easier (if at all possible) to get the correct submit handler to run that way. Using a select box with an extra hidden button for hooking in its submit handler relies on a workaround for IE and will break as soon as there's more than one AJAXed selectbox or the buttons are reordered.
The only downsides to the code I'll post soon is that it has to temporarily negate the "#required" flag on all form elements to pass validation - that should not be a problem as the contents are not actually submitted to the DB - and that I've not yet put in a way to handle what happens if the button is clicked when JavaScript is disabled (it currently updates the form and ignores the selected snippet). Trying to find a nice way of avoiding that.
#17
Great news, thanks. Appreciate you taking the time to work on this. Look forward to applying the patch. =)
#18
I got it to work when JavaScript is disabled as well.
D6 FAPI could use and improvement here or there.... but that would probably break all the modules which have now adapted to this 'funky' behavior...
Well, at least I've got a pretty good picture of how it all fits together now hehe.
If you don't mind, I'm not going to comment on all the changes I made since the last patch. It's now 3am here and I need some sleep.
I'll also get back to you about co-maintaining the module. Thanks for the offer!
#19
Tested and committed: http://drupal.org/cvs?commit=321750
Thanks so much for your work. I need to test if this fixes #521400: Selecting a snippet breaks teaser splitter - need to rebuild body form field using node.module function too. If it does, I'll roll a 6.x-1.0 release. =)
#20
Automatically closed -- issue fixed for 2 weeks with no activity.