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.
Background:
This issue is part of the task to update the hook_help texts of the modules for Drupal 8:
#1908570: [meta] Update or create hook_help() texts for D8 core modules
Tasks:
- review if the text in filter_help() is up to date for Drupal 8
- test the links embedded in the text
- update d.o. docs at https://drupal.org/documentation/modules/filter
Comments
Comment #1
ifrikComment #2
ifrikComment #3
batigolixHere is a patch to get us started. It fixes the urls and updates the namesof the admin page to Text formats & Editors
As a results of the wysiwyg editors in D8 there are quite some changes to the filter module, so this help text needs quite a bit of work
Comment #4
jhodgdonCould we also update the "for more information" section? It still says "handbook" and does not conform to the template.
Thanks!
Comment #5
batigolixA first draft for completely revised text:
Comment #6
jhodgdonI think I like the current patch's first sentence better than the new proposal: "The Filter module allows administrators to configure text formats." is the latest patch. The new proposal says "provides" which kind of implies that the Filter module is providing the text formats, while the old text implies (more accurately) that it provides a UI for admins to create the formats.
So, I think I would prefer to keep the latest patch's About section -- it's more concise and I think gets across the same information in less space, while also being more accurate.
The rest of the new proposal looks pretty good though... Let's see, a few comments (and it may also need some proofreading):
- I don't think we should talk much about editors. Just mention that they can be associated with text formats by using the "Text Editor" module (core/modules/editor) and link to the help there.
- I am not sure the line about "Make sure that the filter "Limit allowed HTML tags" is always enabled to prevent users from entering malicious code. " is a good idea in general. Probably it should just say to make sure this is enabled on text formats that untrusted users can use?
- The section on "Making formats available in fields" is a great addition. Maybe the heading could be "Making text formats available for field editing" though, and I think "you must choose" should be qualified by "if you want a field to use a text format"? And does it really only apply to "long text"? It's possible it would apply to other fields, and it's also not clear to me that they have to be multi-line (so that last thing about choosing a text format should probably say "when creating or editing data in a field that has text formats enabled").
Looking pretty good!
Comment #7
batigolixPatch contains first version of revised text that reflects changes in Filter module for Drupal 8
It addresses the point made in #6
Suppose a careful review is necessary because the numerous changes lead to errors.
Another interesting problem raised by this module is that the name of the configuration page changes depending on enabled modules. Without text editor the link and page title of the admin page is "Text formats". When text editor is enabled it changes to "Text formats and editors page",
Are we going to try to follow that in the help text?
Comment #8
jhodgdonThat's a good question about the name of the config page. How about if we call it the "Text formats" page and the first time it is mentioned, put in parentheses something like "(the name of this page becomes "Text formats and editors" if the Text Editor module is enabled)". Thoughts?
So... I started again at the top and read through the help text as it would be after applying this patch. I have a few suggestions:
a) In the About, we say something about "content and comments". I think we should instead say "text entered in the site" or something like that, because text formats can be used in all sorts of different places on the site.
b) First sentence of Managing bullet point: "You can create and edit Text formats" -- I don't think "Text" should be capitalized. It isn't elsewhere.
c) First sentence of the Roles bullet point: "You can define which users will be able to use the format by selecting roles.". I think "the" should be "each", because to use "the" you need to be already talking about one specific text format, and you aren't here.
d) Maybe the Roles header should be "Assigning roles to formats" not "Assigning formats to roles"?
e) The help text sometimes uses the word "format" and sometimes the phrase "text format". I think it should standardize on using "text format" everywhere.
f) I think the Assigning a text editor bullet point should not provide all those details. Leave those to the other module. Instead, say something like "If you have the ... module enabled, you can assign an editor to each text format. See the ... module help for more details."
g) When talking about the text that is being filtered, sometimes it is called text and sometimes it is called content. I think we should standardize on calling it "text", because "content" is a more generic term (could include images, files, etc.) and "text" is specific to what we're really talking about here.
h) In the Applying bullet point: punctuation: after "e.g.", you need to have a comma. But it's even better to not use "e.g." and instead say "For example".
i) In Applying: "To decide which HTML tags, codes, and other input is allowed in the content you enable filters.". Um. So for one thing, you are not determining what *input* is allowed, since above it says you are filtering at output time. And for another thing, you are not "deciding" by "enabling filters". You decide on your own, and then you put that decision into practice by enabling filters. So maybe: "Each text format is composed of a list of filters." would be clearer and more accurate, given that we've already explained some of the other stuff above?
j) Remove redundant information. The information about filters affecting output appears twice in the help text, for instance.
k) I don't think "Applying filters to text" is the right heading for that bullet point. Really it's talking about setting up a text format. To me "applying filters" sounds like when the text format is actually used to filter text.
I stopped here because it didn't look like the points I made in #6 were actually addressed.
Comment #9
batigolixThanks for the review.
This patch addresses the points in #6 and the remaining points in #6
I removed the bullet point about the Text Editor from the Uses section and included one reference to it in the About section.
Regarding #8 g) : I couldnt find the redundant information. Can you specify this?
Comment #10
jhodgdonLooking better! A few minor things remain:
a) In About, the 2nd sentence starts "A text format..." and then the third (referring to text formats) starts "They". I found the "they" confusing. Either both sentences need to be singular, both need to be plural, or the 3rd sentence needs to start with "Text formats". Your choice. :) Or better yet, those two sentences are pretty short -- they could be combined into one?
b) The redundant information I was referring to: in "Selecting filters":
"generally either adds, removes, or transforms elements within user-entered text before it is displayed." ... " but instead, modifies it temporarily before it is displayed."
===> I think that this whole paragraph could be rewritten to be more concise.
c) Needs a serial comma: "... such as content, a comment or a user account."
d) I had a small disconnect when I went from the "Making text formats available for field editing" section to the "Choosing a text format". That second section refers to when the text format is "enabled", but that terminology is not used in the previous section... actually I think the text in the "Making" section is just confusing -- I had to go back and read it again to understand what it was even referring to.
So... Maybe we should call that first section "Enabling text formats for field editing" and rewrite it something like this:
In the field settings for a field that supports text formats (such as Long Text), you can enable the use of text formats by choosing "Filtered text (user selects text format)" in "Text processing option". [and then have the standard text about the field and field UI modules]
Thoughts? I think that (much shorter) text captures all of the information that is needed. Also it leaves open the idea that a text format could apply to other fields, and I don't think we really need to get into the details of enties, fields, and field UI in this help, do we?
Comment #11
batigolixPatch addresses point made in #10
Comment #12
jhodgdonI think this is great!
I noticed
which I don't think needs to be in the t() in the Enabling text formats for field editing uses item any more (the text referencing it was removed).
But that's minor. We can remove that, and then this should be ready for a manual test:
- Verify that all the links work
- Verify that all mentions of pages/text within the UI match what is seen in the UI
- Verify that the formatting is OK.
Comment #13
batigolixThanks. I removed the variable.
Comment #14
batigolixComment #17
batigolixNew attempt with the same patch
Comment #18
batigolixNeeds review . See #12
Comment #19
jhodgdonI gave this a manual test today, and it all looks good except:
a) Uses / Enabling text formats for field editing
This says that the setting to enable text formats is
But the screen just says "Text processing". Probably it should just say
... under "Text processing".
b) Uses / Choosing
As long as we're fixing (a), maybe we should fix this too even though it is minor.
The text says "the text format widget " but on the screen it is actually "Text format", and maybe it should just say something like "users can select the format under the field from the Text format select list"?
Comment #20
batigolixPatch fixes point from #19
Comment #21
jhodgdonThanks! I think this is finally done. :)
Comment #22
alexpottCommitted 7878e61 and pushed to 8.x. Thanks!