At /admin/config/user-interface/forward under the "Forward form" fieldset, there's a fieldset for the "Personal message field". It's got a setting called "Allowed HTML tags". That should only be visible if both the 'Personal message' field is something other than 'Hidden' AND if the 'Allow HTML in personal messages' checkbox is checked.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 2969337-6.after-checked.png | 95.2 KB | dww |
| #6 | 2969337-6.after-unchecked.png | 52.76 KB | dww |
| #6 | 2969337-6.patch | 1.2 KB | dww |
| #2 | 2969337-2.before.png | 118.77 KB | dww |
Comments
Comment #2
dwwComment #3
dwwNot sure which #states syntax is clearer.
In #2 I'm using AND between conditions, and specifying what has to happen for it to be visible.
Here, I'm using OR between conditions (thanks to more arrays!), and specifying what has to happen for it to be invisible.
This one is at least shorter and a smaller diff.
#2 is maybe more explicit? Up to the maintainer to pick. Both provide the same behavior as seen in the screenshots.
Cheers,
-Derek
Comment #4
john.oltman commentedIn the "unchecked" case, the phrase "all tags not allowed below" no longer makes sense since tags are no longer displayed below. If you are going to conditionally display the tags field, my suggestion would be to change the descriptions as follows.
[x] Allow HTML in personal messages
If not enabled, any HTML in the message will be converted to plain text.
Allowed HTML tags
[ the input field is here ]
List of tags (separated by commas) that will be allowed if HTML is enabled above. XSS and tags not allowed will be filtered out. Defaults to:
However, this will mean existing translations for strings in Forward admin will now be out of sync. Therefore, I lean towards making this a "Closed (won't fix)" since the upside is minimal.
Comment #5
dwwOh crap, good point. :/ This module is already stable, and changing these descriptions would break translations. Whoops.
Let's postpone this for a future text-breaking major release (either 8.x-3.x or 9.x or whatever), and yeah, fix the descriptions accordingly.
Thanks/sorry,
-Derek
Comment #6
dwwUpon further consideration, let's just kill that description entirely. It's sort of confusing for end users, and I think the checkbox title speaks for itself. Especially in combination with the other setting (once revealed). This wouldn't break translations, since no strings are changed. There'd be extra translated text not being used, but that's better than the alternative.
Thoughts?
Comment #7
john.oltman commentedComment #8
john.oltman commentedLet's postpone. Whether somebody decides to enable the option could be influenced by the fact that there is a tag filter available. If I can't see that the tags can be filtered, I might not choose to enable it.
Comment #9
dwwTotally fair. I'll probably apply the patch locally on the sites I care about, but I completely respect your decision to leave this alone upstream.
Thanks,
-Derek
Comment #10
john.oltman commented