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

Files: 
CommentFileSizeAuthor
#20 interdiff.txt2.47 KBbatigolix
#20 update-hook-help-for-filter-module-2041701-20.patch6.49 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 63,756 pass(es).
[ View ]
#17 update-hook-help-for-filter-module-2041701-13.patch6.49 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 63,195 pass(es).
[ View ]
#13 update-hook-help-for-filter-module-2041701-13.patch6.49 KBbatigolix
FAILED: [[SimpleTest]]: [MySQL] 63,154 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
#11 update-hook-help-for-filter-module-2041701-11.patch6.56 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 63,008 pass(es).
[ View ]
#11 interdiff.txt6.19 KBbatigolix
#9 interdiff.txt8.61 KBbatigolix
#9 update-hook-help-for-filter-module-2041701-9.patch7.08 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 59,887 pass(es).
[ View ]
#7 interdiff.txt8.31 KBbatigolix
#7 update-hook-help-for-filter-module-2041701-7.patch7.41 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 59,397 pass(es).
[ View ]
#3 update-hook-help-for-filter-module-2041701-3.patch5.96 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 58,609 pass(es).
[ View ]

Comments

ifrik’s picture

Assigned:Unassigned» ifrik
ifrik’s picture

Assigned:ifrik» Unassigned
batigolix’s picture

Component:documentation» filter.module
Status:Active» Needs review
StatusFileSize
new5.96 KB
PASSED: [[SimpleTest]]: [MySQL] 58,609 pass(es).
[ View ]

Here 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

jhodgdon’s picture

Status:Needs review» Needs work

Could we also update the "for more information" section? It still says "handbook" and does not conform to the template.

Thanks!

batigolix’s picture

Assigned:Unassigned» batigolix
Issue summary:View changes

A first draft for completely revised text:

The Filter module provides text formats that are applied to user-generated content before it is displayed on the web page. By enabling certain filters in text format you define which HTML tags, codes, and other input is allowed in the body of pages, comments and other content. They defend your web site against potentially damaging input from malicious users. Furthermore the text format defines if a visual text editor will be avaiable. To use For more information, see the online documentation for the Filter module.

Managing text formats

You can create and edit Text formats on the Text formats and editors page. One format is included by default: Plain text (which removes all HTML tags). Additional formats may be created during installation. You can create a text format by clicking "Add text format".

Assigning formats to roles

You can define which users will be able to use the format by selecting roles. To ensure security anonymous and untrusted users should only have access to text formats that restrict them to either plain text or a safe set of HTML tags. This is because HTML tags can allow embedding malicious links or scripts in text. More trusted registered users may be granted permission to use less restrictive text formats in order to create rich content. Improper text format configuration is a security risk.

Enabling a visual text editor

By choosing a text editor you can provide users with a visual text editor. The default visual text editor is CKEeditor. If a text editor is chosen you can configure the toolbar for the editor. You can add and remove buttons from the Active toolbar by dragging and dropping them, and additional rows and groups can be added to organize the buttons.

Applying filters to text

To decide which HTML tags, codes, and other input is allowed in the content you enable filters. Each filter is designed for a specific purpose, and generally either adds, removes, or transforms elements within user-entered text before it is displayed. A filter does not change the actual content, but instead, modifies it temporarily before it is displayed. One filter may remove unapproved HTML tags, while another automatically adds HTML to make URLs display as clickable links. Make sure that the filter "Limit allowed HTML tags" is always enabled to prevent users from entering malicious code. Set the Filter processing order to ensure that all filters are applied in the right order.

Each filter can have additional configuration options. E.g. for the "Limit allowed HTML tags" filter you need to define the list of HTML tags that the filter leave in the text.

Making formats available in fields
Text formats can be used for processing the content of Long text fields that are attached to an entity type such as content, a comment or a user account. If you create a new content type and add a Long text field, you must choose "Filtered text (user selects text format)" in "Text processing option". This will ensure that the text formats and the visual text editor will be available when entering content in that particular field.

Choosing a text format

Users with access to more than one text format can use the Text format widget to choose between available text formats when creating or editing multi-line content.

jhodgdon’s picture

I 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!

batigolix’s picture

Status:Needs work» Needs review
StatusFileSize
new7.41 KB
PASSED: [[SimpleTest]]: [MySQL] 59,397 pass(es).
[ View ]
new8.31 KB

Patch 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?

jhodgdon’s picture

Status:Needs review» Needs work

That'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.

batigolix’s picture

Status:Needs work» Needs review
Parent issue:» #1908570: [meta] Update or create hook_help() texts for D8 core modules
StatusFileSize
new7.08 KB
PASSED: [[SimpleTest]]: [MySQL] 59,887 pass(es).
[ View ]
new8.61 KB

Thanks 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?

jhodgdon’s picture

Status:Needs review» Needs work

Looking 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?

batigolix’s picture

Status:Needs work» Needs review
StatusFileSize
new6.19 KB
new6.56 KB
PASSED: [[SimpleTest]]: [MySQL] 63,008 pass(es).
[ View ]

Patch addresses point made in #10

jhodgdon’s picture

Status:Needs review» Needs work

I think this is great!

I noticed

'!entity_help' => \Drupal::url('help.page', array('name' => 'entity'))

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.

batigolix’s picture

Status:Needs work» Needs review
StatusFileSize
new6.49 KB
FAILED: [[SimpleTest]]: [MySQL] 63,154 pass(es), 2 fail(s), and 1 exception(s).
[ View ]

Thanks. I removed the variable.

batigolix’s picture

Assigned:batigolix» Unassigned

Status:Needs review» Needs work

The last submitted patch, 13: update-hook-help-for-filter-module-2041701-13.patch, failed testing.

The last submitted patch, 13: update-hook-help-for-filter-module-2041701-13.patch, failed testing.

batigolix’s picture

Status:Needs work» Needs review
StatusFileSize
new6.49 KB
PASSED: [[SimpleTest]]: [MySQL] 63,195 pass(es).
[ View ]

New attempt with the same patch

batigolix’s picture

Needs review . See #12

jhodgdon’s picture

Status:Needs review» Needs work

I 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

in "Text processing option"

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"?

batigolix’s picture

Status:Needs work» Needs review
StatusFileSize
new6.49 KB
PASSED: [[SimpleTest]]: [MySQL] 63,756 pass(es).
[ View ]
new2.47 KB

Patch fixes point from #19

jhodgdon’s picture

Status:Needs review» Reviewed & tested by the community

Thanks! I think this is finally done. :)

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 7878e61 and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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