Hi,

When "Enabled by default" is disabled was not working for me. Tried with Firefox 2 and 3.

I managed to write a fix that seems to be working for me. Please, see attached patch.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

markus_petrux’s picture

BTW, tested with one or more WYSIWYG editors enabled in the same page.

HTH+

sun’s picture

Please note that I'm reconsidering the whole default status configuration in #322433: Replace default editor status option(s) with intelligent logic. Depending on the resulting implementation, this patch might need to be reconsidered, too.

emilyf’s picture

subscribing
thanx markus_petrux for the patch -- works great

Murz’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, patch works great! Waiting for commiting to HEAD

sun’s picture

Status: Reviewed & tested by the community » Needs work

Unfortunately, this patch has some major flaws.

1) Drupal.settings.wysiwyg.status should not be altered at runtime. Despite being completely misplaced in the current JS settings (should be per input format/profile instead), this variable stores the default state (upon loading) only.

2) The actions performed in Drupal.behaviors.attachWysiwyg() and Drupal.wysiwygAttachToggleLink() should not be mixed and kept atomic. It has to be possible to completely disable (not output) the toggle link at all, but the current patch moves Drupal.wysiwygAttach() into the toggle link attachment function.

sun’s picture

First take on this one.

sun’s picture

Assigned: Unassigned » sun
Status: Needs work » Needs review
FileSize
15.11 KB

I need a very thorough review and extensive testing of attached patch ASAP. This issue is the last blocker for a 0.5 release.

What this patch does:

- Defines and stores the default state of each editor, i.e. per-editor/input format.
- Ensures that the enable/disable is always displayed/updated/removed accordingly, i.e. taking default states as well as the last editor state into account.
- Moves the complete logic for discovering whether an editor actually needs to attached or detached into wysiwyg.js core functions, i.e. simplifying editor integration JavaScripts.

I've tested this extensively, but would like have at least one other confirmation.

RobLoach’s picture

Adding to my ever-growing list of things to review. Will test it tomorrow.

markus_petrux’s picture

Hi,

Patch in #7 seems to be working nicely here. It is looking great. :)

What I found a bit confusing is that fact that it is missing is the ability to update the user preference somehow. Related to #322433: Replace default editor status option(s) with intelligent logic

It confused me because the user I'm using to test has $user->wysiwyg_status defined, so it takes precedence over the editor profile setting when "Allow users to choose default" is enabled.

sun’s picture

Status: Needs review » Fixed

Thanks, committed to all branches.

RobLoach’s picture

Markus got to the review before I did. And yes, this is awesome.

Status: Fixed » Closed (fixed)

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

mpaler’s picture

Does this include the 5.x branch?

wickwood’s picture

Version: 6.x-1.x-dev » 6.x-2.0-alpha1
Status: Closed (fixed) » Active

Hello,

I'm experiencing this issue again. I know this was "fixed", but my users who should not see the link are.

I have under basic setup in Wysiwyg profile:
Enabled by default - checked
Allow users to choose default - unchecked
Show enable/disabled rich text toggle link - unchecked.

Is there another permission or some other configuration I should look for?

Thanks for you help in advance.

sun’s picture

Status: Active » Closed (fixed)

Please update to the latest development snapshot before re-opening this issue.