The Allow users to choose default settings does not currently work.

Because of the logic the wysiwyg_status field on the user form never displays.
$profile is always an empty object.

<?php
    $profile = new stdClass;
    if (isset($profile->settings['user_choose']) && $profile->settings['user_choose']) {
      $form['wysiwyg'] = array(
        '#type' => 'fieldset',
        '#title' => t('Wysiwyg Editor settings'),
        '#weight' => 10,
        '#collapsible' => TRUE,
        '#collapsed' => TRUE,
      );
      $form['wysiwyg']['wysiwyg_status'] = array(
        '#type' => 'checkbox',
        '#title' => t('Enable editor by default'),
        '#default_value' => isset($user->wysiwyg_status) ? $user->wysiwyg_status : (isset($profile->settings['default']) ? $profile->settings['default'] : FALSE),
        '#return_value' => 1,
        '#description' => t('If enabled, rich-text editing is enabled by default in textarea fields.'),
      );
      return array('wysiwyg' => $form);
    }
?>
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markus_petrux’s picture

Any idea on how should it be implemented? Would it be fine to using "data" in user records? Different storage method?

Andrew Schulman’s picture

subscribe

Agileware’s picture

Status: Active » Needs review
FileSize
2.38 KB

It should be fine to use data to store the info or do it the way it is currently.

The problem is that no wysiwyg profiles are loaded into the $profile variable before it checks the user_choose setting.
Another problem is that this would provide a single setting but there can be multiple wysiwyg profiles with different user_choose settings.

Here is a quick patch that should resolve this, giving them a checkbox for each input filter they have access to.
I haven't really had time to test it but that's the sort of thing we need.

TwoD’s picture

FileSize
2.71 KB

Thanks for the patch!

I've added a period to the end of #description and changed the #default_value handling a bit.
If an editor profile didn't have user_choose when the user made their choice, but it does now, the profile's default state will be used until it is overridden.

j_mcc’s picture

Any chance of seeing this added to a new update soon, or at least a dev release?

TwoD’s picture

This is marked as "Needs review", so if someone is willing to test the patch and confirm that it works (and is a practical solution, at least until #322433: Replace default editor status option(s) with intelligent logic gets fixed), we can get this rolled into -dev.

hankp98072-’s picture

subscribe

mstef’s picture

works!

mathieu’s picture

Thanks for the patch - works for me too.

mgriego’s picture

Status: Needs review » Reviewed & tested by the community

Works for me as well. Looking forward to seeing this added into a release soon.

gbowman’s picture

This worked for me, it didn't do what I expected it to do but allowed me to achieve what I wanted!

TwoD’s picture

@gbowman, what did you expect it to do? If I learn what users expect, maybe I can create patches that meet those expectations better. ;)

gbowman’s picture

I'm not sure now, been a while since I wrote that. I think I was looking for it to allow me to assign an editor to particular users (namely Anonymous). I think originally I was looking to Disable rich-text for guests to make the comments form simpler.

Thinking about it though the way it is set now seems most logical... simple text box as default and a wysiwyg editor as default for my account.

colan’s picture

Subscribing.

hanoii’s picture

subscribe

hanoii’s picture

applied the patched and worked fine, +1 to be added to -dev

gr33nman’s picture

This patch worked. It also fixed a white screen of death I was getting when I tried to /user/1/edit after enabling wysiwyg in Drupal 6.19.

This does not do what I expected it to do: allow a user to select via radio-button the preferred wysiwyg editor to use as default in the body textarea when creating a new story/page. I have three available input formats: Filtered html (no wysiwyg editor applied), FCK_Editor_Full (for full FCK), and TinyMCE_Full (obvious). All three still show up and Filtered html is still selected as default when I create a new story or page. I would like there to be a filtered default for authenticated comments that has no editor, but users with editor and author privileges should be able to select their preferred wysiwyg.

I guess I'm not exactly sure what this patch changes, other than to put selectable wysiwyg checkboxes in the user my account edit area.

gr33nman’s picture

Status: Reviewed & tested by the community » Needs review
TwoD’s picture

Status: Needs review » Reviewed & tested by the community

@gr33nman, the patch is meant to allow users to choose whether an editor profile for a format is enabled by default or not - when that format is switched to during editing, not which format is selected by default. Use the Better Formats module for that.

Sun and I briefly discussed this topic a while ago and we want to implement it a different way later. Though since this patch does make the checkbox in the profile settings do something useful for users who don't want editors by default, I think we should put it in now.

gr33nman’s picture

Okay - thank you for the explanation and the suggestions. I greatly appreciate it.

mstef’s picture

Hasn't been committed to 2.2?

Michelle’s picture

Thanks for the patch. This is critical for me since WYSIWYG doesn't work in Panels and even clicking the option to turn it off doesn't make the text area come back. I realize that this is moving in another direction but it seems to me that this patch is needed in the mean time. It works fine. Why not commit it?

Michelle

sun’s picture

Status: Reviewed & tested by the community » Needs work
+++ wysiwyg.module	24 Apr 2010 18:40:01 -0000
@@ -620,8 +620,20 @@ function wysiwyg_profile_load_all() {
     // @todo http://drupal.org/node/322433

What's this @todo? Obsolete?

+++ wysiwyg.module	24 Apr 2010 18:40:01 -0000
@@ -620,8 +620,20 @@ function wysiwyg_profile_load_all() {
+    $profiles = wysiwyg_profile_load_all();
...
+    foreach ($profiles as $format => $profile) {

We can directly use the return value of wysiwyg_profile_load_all() instead of assigning the $profiles variable.

+++ wysiwyg.module	24 Apr 2010 18:40:01 -0000
@@ -620,8 +620,20 @@ function wysiwyg_profile_load_all() {
+    $user_formats = filter_formats();

The D7 patch will have to look slightly different, and will also need a horrible upgrade path for {users}.data records... :(

Frankly, I'd almost prefer - if we really have to do this right now - to create a new {wysiwyg_user} db table, two columns uid and format, no primary key; deleting and re-inserting all rows per uid upon user account update.

+++ wysiwyg.module	24 Apr 2010 18:40:01 -0000
@@ -620,8 +620,20 @@ function wysiwyg_profile_load_all() {
+      if (isset($profile->settings['user_choose']) && $profile->settings['user_choose'] && array_key_exists($format, $user_formats)) {

The first two conditions are !empty().

The last condition should be isset().

+++ wysiwyg.module	24 Apr 2010 18:40:01 -0000
@@ -620,8 +620,20 @@ function wysiwyg_profile_load_all() {
+    if (count($options)) {

Let's use !empty() here.

+++ wysiwyg.module	24 Apr 2010 18:40:01 -0000
@@ -630,11 +642,11 @@ function wysiwyg_user($type, &$edit, &$u
         '#title' => t('Enable editor by default'),
...
+        '#description' => t('For each of the input formats, select whether or not rich-text editing is enabled by default in textarea fields.'),

Let's drop the description and make the title slightly more descriptive; i.e.:

"Text formats enabled for rich-text editing"

Powered by Dreditor.

TwoD’s picture

Status: Needs work » Needs review
FileSize
3.25 KB
3.25 KB

I did not have time to test an upgrade so I've not looked into a possible upgrade path yet.
Added an extra parameter to wysiwyg_user_get_status() so its possible to get the status for someone other than the logged in user, but in D6 filter_formats() will still only present the formats available for the logged in user instead of those available for another user when editing their account.

Please correct me if the user of filter_formats() in D7 is a security risk. (If a user has permissions to edit another user, I don't think it'd be dangerous to show them which formats that user can access, even if the editing user can't access them. They'd need special permissions to do something with those formats anyway.)

This took longer than I anticipated and I have to rush this and go to work now so I might have missed something.

sun’s picture

Status: Needs review » Needs work

Thanks! Let's focus on D6 for now, and forward-port later on.

+++ wysiwyg.module
@@ -610,46 +610,62 @@ function wysiwyg_profile_load_all() {
- * Implementation of hook_user().
+ * Implements hook_form_FORM_ID_alter().
...
-function wysiwyg_user($type, &$edit, &$user, $category = NULL) {
...
+function wysiwyg_form_user_profile_form_alter(&$form, &$form_state, $form_id) {

mmm, we should keep the hook_user() implementation, because alter hooks are not intended to add things, only to alter already existing things.

+++ wysiwyg.module
@@ -610,46 +610,62 @@ function wysiwyg_profile_load_all() {
+    $form['wysiwyg'] = array(
+      '#type' => 'fieldset',
+      '#title' => t('Wysiwyg Editor settings'),
+      '#weight' => 10,
+      '#collapsible' => TRUE,
+      '#collapsed' => TRUE,
+    );

btw, I'd be fine with dropping that fieldset, too.

+++ wysiwyg.module
@@ -610,46 +610,62 @@ function wysiwyg_profile_load_all() {
-function wysiwyg_user_get_status($profile) {
...
+function wysiwyg_user_get_status($profile, $account = NULL) {

Let's leave the addition of $account for later... not needed right now (and as you mentioned, not really possible prior to D7).

+++ wysiwyg.module
@@ -610,46 +610,62 @@ function wysiwyg_profile_load_all() {
+function wysiwyg_user_presave(&$edit, $account, $category) {
+  if (isset($edit['wysiwyg_status'])) {
+    $edit['data']['wysiwyg_status'] = $edit['wysiwyg_status'];
+  }
+}

At least in D6, all properties contained in $edit, resp. in the submitted form values, are automatically taken over into $user->data, so I don't think that this presave implementation is necessary.

+++ wysiwyg.module
@@ -610,46 +610,62 @@ function wysiwyg_profile_load_all() {
+  if (!empty($profile->settings['user_choose']) && isset($account->data['wysiwyg_status'][$profile->format])) {
+    $status = $account->data['wysiwyg_status'][$profile->format];

All serialized data in {users}.data is automatically "unpacked" onto the $account object, so the previous patch was correct; the account settings are contained in $user->wysiwyg_status.

Powered by Dreditor.

TwoD’s picture

The DRUPAL-6--2 patch is screwed up, it shouldn't have those changes at all. I'll re-diff that ASAP.

TwoD’s picture

Let's try this one instead. I wish I had a patch viewer, and maybe even Dreditor on my phone... ;)

sun’s picture

Status: Needs work » Needs review

Seems like we're still missing the {wysiwyg_user} table?

+++ wysiwyg.module
@@ -628,42 +628,41 @@ function wysiwyg_profile_load_all() {
+      // Only show formats a user has access to use that allow user_choose.

Minor: This comment reads a bit odd, let's re-phrase it.

Powered by Dreditor.

TwoD’s picture

{wysiwyg_user}? Didn't we talk about implementing that later and fixing what is there first?

sun’s picture

Assigned: Unassigned » sun

Frankly, I'd almost prefer - if we really have to do this right now - to create a new {wysiwyg_user} db table, two columns uid and format, no primary key; deleting and re-inserting all rows per uid upon user account update.

Given that D7 uses a new text format identifier format, and we'll have to maintain these user preferences somehow, we should really store it in an own, custom database table. Should actually be 5 minutes of work...

sun’s picture

Assigned: sun » Unassigned
FileSize
7.21 KB

Like this?

TwoD’s picture

Still on my phone so can't do a proper review until morning...

+ i f ( d b _ t a b l e _ e x i s t s ( ' w y s i w y g _ u s e r ' ) ) {
+ d b _ c r e a t e _ t a b l e ( $ r e t , ' w y s i w y g _ u s e r ' , a r r a y (

If table exists -> create it again?

The odd comment is still there, how about simply "// Only show formats that have user_choose."?

I like the "lazy-loading" of statuses! =)

TwoD’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
6.75 KB

Some minor fixes and we're good to go!

+++ wysiwyg.install	4 Jan 2011 00:35:36 -0000
@@ -177,3 +217,52 @@ function wysiwyg_update_6200() {
+  if (db_table_exists('wysiwyg_user')) {

Added missing "!".

+++ wysiwyg.module	4 Jan 2011 00:45:56 -0000
@@ -626,44 +626,65 @@ function wysiwyg_profile_load_all() {
+    foreach ($wysiwyg_profile_load_all() as $format => $profile) {
+      // Only show formats a user has access to use that allow user_choose.

Fixed function call and changed comment to "// Only show profiles that have user_choose enabled.", the user access part happens in filter_formats() so not relevant here.

The table gets created both on clean install and update, populates and gets destroyed on uninstall. Settings now persist when set for both the current user and other users (though as noted earlier, current user can only see formats they have access to when editing other users).

Powered by Dreditor.

sun’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks for reporting, reviewing, and testing! Committed to D6.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Now we need to quickly forward-port this patch to D7, as we need to release 7.x-1.0 this week.

TwoD’s picture

Status: Patch (to be ported) » Needs review
FileSize
7.28 KB

D7 port.

Please remove the drupal_set_message lines... generated patch from my (otherwise identical) debug branch...

sun’s picture

sun’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
FileSize
5.35 KB

Committed to HEAD.

Follow-up fixes for D6. Do we need to provide db updates for the 24h time-frame?

TwoD’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Needs review » Reviewed & tested by the community

Ah, nice changes. I still have some reading to do over at http://drupal.org/update/modules/6/7
Patch looks good and works perfectly!

Crosspost... Second patch is good too, don't think it needs an update.

sun’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Alright, also committed the follow-up patch for D6.

Marking needs work, because this functionality can be tested through automated unit tests (and we currently have none).

mstef’s picture

What's the status here, exactly? I know this didn't make it into 2.2 - but it's been committed to 6.x dev, right?

TwoD’s picture

Yes, this is all committed to 6.x-2.x-dev and 7.x-2.0.
All that's left are the automated tests.

sun’s picture

Status: Needs work » Fixed

Let's deal with automated unit tests later/elsewhere. This is probably one of the least important things to test. :P

Status: Fixed » Closed (fixed)
Issue tags: -Needs tests

Automatically closed -- issue fixed for 2 weeks with no activity.