Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#7 | wysiwyg-HEAD.default-status.patch | 15.11 KB | sun |
#6 | wysiwyg-HEAD.default-status.patch | 13.12 KB | sun |
wysiwyg.js_.patch.txt | 1.11 KB | markus_petrux |
Comments
Comment #1
markus_petrux CreditAttribution: markus_petrux commentedBTW, tested with one or more WYSIWYG editors enabled in the same page.
HTH+
Comment #2
sunPlease 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.
Comment #3
emilyf CreditAttribution: emilyf commentedsubscribing
thanx markus_petrux for the patch -- works great
Comment #4
MurzThanks, patch works great! Waiting for commiting to HEAD
Comment #5
sunUnfortunately, 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.
Comment #6
sunFirst take on this one.
Comment #7
sunI 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.
Comment #8
RobLoachAdding to my ever-growing list of things to review. Will test it tomorrow.
Comment #9
markus_petrux CreditAttribution: markus_petrux commentedHi,
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.Comment #10
sunThanks, committed to all branches.
Comment #11
RobLoachMarkus got to the review before I did. And yes, this is awesome.
Comment #13
mpaler CreditAttribution: mpaler commentedDoes this include the 5.x branch?
Comment #14
wickwood CreditAttribution: wickwood commentedHello,
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.
Comment #15
sunPlease update to the latest development snapshot before re-opening this issue.