Port to 6.x
| Project: | Wysiwyg |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Drupal 6 compatible version doesn't seem to work at my test site at all...
It starts at the install stage ("warning: Missing argument 1 for wysiwyg_editor_menu() in /var/www/html/beta.gewiss.si/sites/all/modules/wysiwyg/wysiwyg_editor.module on line 13." message).
After, there are only 2 items in the admin/by-module section:
* Configure permissions
* Get help
(Wysiwyg module configuration is missing...)
I've checked the code with Coder ("code review"), and it has thrown out a bunch of error messages. I wonder how this module can be declared as "Drupal 6" compatible - as it is - at all... ;-)
So, to not only complain but also try to contribute, I've spent (quite) some time trying to make Coder more happy and Wysiwyg module working. I've managed to get rid of error messages and to get the admin screens working (a profile can be created and edited later, altough "Wysiwyg Editor profile has been created." message is always appearing - even after editing an existing profile...). All of the changes are in the patch attached.
Now, "the only" residual issue is that the module simply doesn't seem to work (there are no user settings, altough "Allow users to choose default" option is activated, and text areas are as they were before, without TinyMCE and any link to enable/disable it...). There are no errors logged, and I don't know how to continue. Any ideas about how to continue?
#1
Sorry, no support for D6 yet. Wysiwyg is still under heavy development.
#2
But, there is a Drupal 6 version published... What is the purpose of it than?
By the way, I've done some more code changes, and it seems I've managed to get so far that javascript part should be executed (but, also doesn't work well). I don't know how to proceed with javascript debugging (troubelshooting) though...
I can provide latest patch (diff) if anyone is interested.
#3
That's a development snapshot only, which is also not listed on the project page.
Sure, if you have something to contribute, I'll be glad to review patches. However, 5.x is still under heavy development, so you should be sure to create a patch against latest code from CVS.
#4
Is there any simple way to get these files (as a package)? Or, can I simply provide all of my files, and you make a diff to the version you need?
#5
If you wait approx. 12 hours, the latest code will be available in the development snapshot. Be sure that its date is newer than now.
Other than that, I highly recommend to learn how to work with CVS, so you can provide proper patches against the latest code: http://drupal.org/handbook/cvs and http://drupal.org/patch
#6
#7
That said, here is patch for D6. Applies against current code in CVS only. (or against a new devel snapshot, approx. available in 12 hours)
#8
LUTi, it would be great if you could test that patch.
#9
OK, here it is...
The easy part first:
Just to be complete - your patch contains an error ("Hunk #1 FAILED at 2." for "wysiwyg_editor.info" file is reported). It seems you've edited a patch manually, and the last 3 equal lines are missing... I've applied that part manually, anyway.
Coder has some complains, but nothing crucial, I'd say... It would be nice to find a way how to properly insert a string into query as a comma separated numbers though (coder insists on single quotes usage around the string, but is breaks the query, as it is actually not used within our queries as a string).
And now to serious things:
I've successfully installed wysiwyg and created a profile (for my admin role), which I've also edited later (now also the create / update message seems to be OK).
But, when I've tried to edit a node, the following error message was displayed:
"warning: array_merge() [function.array-merge]: Argument #1 is not an array in .../wysiwyg_editor.module on line 510."
Otherwise, TinyMCE seems to be somehow present (enable / disable rich-text link works, altough it restores just one textarea and not 2 - separate for the teaser and separate for the body - as without wysiwyg; resize also seems to work well), but there are no buttons (the button area is just a few px high...).
Taking a closer look, I've noticed that "Allow users to choose default" option seems to work (if enabled, user can enable or disable default), but everything else somehow doesn't matter when we come to the textarea display - reach-text editor and enable / disable link are always present, no matter what is chosen...
At user settings, wysiwyg option is displayed (if set so by admin), but "enable" doesn't work ("disabled" is always present when I come to this setting). Despite of that, reach-text editor is always appearing...
So, I see a huge improvement, but still there are many issues:
- at the user screen, setting the option is not accepted
- at textarea (node edit) itself, TinyMCE is always on by default (no matter what is selected where), as well as enable / disable link
- within TinyMCE, buttons are missing (probably due to the "array_merge" error...)
- after reach-text is disabled, not both separate windows (for teaser and body) are properly restored (of course there is also no option if the teaser has to be merged with the body or not...)
#10
About the issue that user settings are not properly saved (as above), there is a typo on user form - instead of (line 247 of wysiwyg_editor.module):
$form['wysiwyg']['wysiwyg_status'] = array(it should be:
$form['wysiwyg']['wysiwyg_editor_status'] = array(#11
I believe the problem with the array_merge error reported (in #9) is that at row 1 we try to merge buttons to a previous (non-existing) row ($init['theme_advanced_buttons'. ($row - 1)] returns NULL at 1st row...). I am attaching my patch which seems to resolve the issue (buttons are displayed with the patch applied, and seems to work as well...).
#12
Resize works only in vertical direction (height adjustment). Is it so by design, or shall it be possible to resize horizontally (width) as well?
And another thought - I can understand there are many advantages if we use TinyMCE v. 2.1 (works quite stable in Drupal 5), but still - as it seems to me many more work will be needed to adjust everything to Drupal 6 differences, wouldn't it make sense to start the development with the latest version available at the moment (v. 3)? As probably some efforts will be wasted later (and time spent alltogether before we will have a production-site "almost ready" version...) due to the TinyMCE changes made in between.
#13
#9: Since you seemed to have problems applying this patch, I've committed the code to HEAD. A new development snapshot should be available shortly.
#9: The PHP warning has been fixed in a subsequent commit.
#9:
The teaser/body separator button is disabled for now, since supporting this Drupal core feature will be a quite hazzle. However, a node edit form should not be displayed with teaser/body separated at all then, which needs to be investigated and fixed.
#10: Wrong user settings variable has been fixed in a subsequent commit.
#11: Should also be fixed now, but using a different approach. It actually seems that there were no buttons enabled for you wysiwyg profile, which might be a bug at another point.
#12: Resizing should only work vertically. We might introduce a setting to support horizontal resizing, too, but definitely not during this port.
#12: (Concurrent) support for TinyMCE 3.x is already on the todo list, but please let us focus on the port now.
Please let me know which bugs you're encountering with the latest development snapshot then.
#14
About the teaser (summary) / body handling, I believe after disabling wysiwyg everything has to be restored exactly as wysiwyg wouldn't be present (installed) at all.
We can not expect from every user to be familiar with Drupal internal teaser / body handling ("break " syntax and usage), as it is now (quite weird and far from being optimal by my opinion - without summary, teaser field contains the 100% duplicate of a body, with separate summary and selected option to show summary in full view as a part of the body teaser is also a partial duplicate of a body, everything before "break"...). The fact is, that we can have 3 situations - no separate summary or separate summary (teaser), and in the second case we can decide weather teaser shall be merged with the body for a full view or not. I (as the average user without many HTML skills - otherwise I probably wouldn't need TinyMCE at all...) would definitely like to see it exactly that way (as I am used to) when I disable reach-text editor!
By the way, I can understand how to handle "no summary" (no "break") and "summary - show summary in full view" situations ("break" somewhere in between the content) in TinyMCE (within a single textarea), but how to have a different summary (teaser) if there are no 2 separete textareas available?
#15
Your #13 solution (instead of my #11) doesn't work. While my solution successfully deals with my case (only 4 buttons selected), your solution causes only the blank page to be displayed... If I have 5 or more buttons, everything works well though. I don't want to insist on my solution at all, nor I claim it is a complete one. But, for the case of less than 5 buttons being selected, your solution has to be somehow improved.
#16
I am observing the situation with TinyMCE module and Drupal. It seems to me that the introduction of javascript in Drupal 6 core textarea handling is causing quite some problems to be successfully "combined" with TinyMCE javascript handling.
Doesn't it mean many of the efforts for TinyMCE 2.1 integration could be wasted for the TinyMCE 3.x integration? I don't know much about the differences between both, I am just asking if it wouldn't maybe be even easier (quicker) to start directly with 3.0 - without resolving some TinyMCE 2.1 integration issues which maybe wouldn't even be present with 3.x...
#17
All possible. However, TinyMCE v3.x has nothing to do with this port and is a completely different issue. It won't make this port easier. So please keep this topic out of this issue and let's focus on the port to D6.
Marking as PNW. However, I won't have time to create a new patch in the next few days...
#18
Thus, unassigning from me
#19
6.x is up and running. Please file a new issue if you find a bug.
#20
Automatically closed -- issue fixed for two weeks with no activity.