Small typo in Drupal\editor\Tests\EditorSecurityTest:

// Enable editor_test.module's hook_editor_xss_filter_alter() implementation
// to ater the text editor XSS filter class being used.

This should be 'alter' instead of ater.

CommentFileSizeAuthor
#5 interdiff-3-5.txt539 bytescs_shadow
#5 editor-2307481-5.patch2.03 KBcs_shadow
#3 editor-2307481-3.patch2.02 KBcs_shadow
#1 editor-fix_typo-2307481-1.patch819 bytescs_shadow
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
Status: Active » Needs review
FileSize
editor-fix_typo-2307481-1.patch819 bytes
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented

Great find.

However, committing just this is rather silly. I noticed that this class uses undocumented class properties. i.e. it used $this>untrusted_user, $this->normal_user, etc., but they're not declared & documented as properties. Could you fix that as well? That makes committing this issue more worthwhile/less of a hassle for core committers.

Thanks!

cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
FileSize
editor-2307481-3.patch2.02 KB

Changes in this patch:

1. Documented class properties $untrusted_user, $normal_user, $trusted_user and $privileged_user.

2. 

// Create 3 users, each with access to different text formats/editors:                                                                        
//   - "untrusted": restricted_without_editor                                                                                                 
//   - "normal": restricted_with_editor,                                                                                                      
//   - "trusted": restricted_plus_dangerous_tag_with_editor                                                                                   
//   - "privileged": restricted_without_editor, restricted_with_editor,                                                                       
//     restricted_plus_dangerous_tag_with_editor,

In comments, its mentioned that the code creates 3 users, but actually 4 users are created. Fixed the number to 4.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Status: Needs review » Needs work
Issue tags: +Quickfix

One nitpick that needs to be fixed before I can RTBC it:

+++ b/core/modules/editor/src/Tests/EditorSecurityTest.php
@@ -46,6 +46,34 @@ class EditorSecurityTest extends WebTestBase {
+   * User with access to Restricted HTML text format, dangerous tags allowed with text editor.

Should wrap at 80 cols.

cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
Status: Needs work » Needs review
FileSize
editor-2307481-5.patch2.03 KB
interdiff-3-5.txt539 bytes

Fixed the line wrap.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Category: Bug report » Task
Status: Needs review » Reviewed & tested by the community

Lovely, thanks!

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Title: Minor typo in Drupal\editor\Tests\EditorSecurityTest » Fix typos and add missing properties in Drupal\editor\Tests\EditorSecurityTest
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed afd7cf9 and pushed to 8.x. Thanks!

  • alexpott committed afd7cf9 on 8.x 
    Issue #2307481 by cs_shadow: Fix typos and add missing properties in...

Status: Fixed » Closed (fixed)

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