Updated: Comment #N

Problem/Motivation

After #2099741: Protect WYSIWYG Editors from XSS Without Destroying User Data was committed it seems the Drupal\editor\Tests\EditorXssFilter\StandardTest unit test class does not run, and therefore does not expose the failing test assertion from the data provider.

Steps to reproduce: `cd path/to/site/core`, `phpunit` - You should see one failure.

Also, try to run this test from either the UI or run-tests.sh - Class not found.

Proposed resolution

Fix the broken namespace in the file, fix the test failure.

Remaining tasks

Fix the failure

User interface changes

None

API changes

None

CommentFileSizeAuthor
#1 2192895.patch730 bytesdamiankloip
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

Status: Active » Needs review
FileSize
730 bytes

This should expose the test failure by running the test.

damiankloip’s picture

Title: Drupal\editor\Tests\EditorXssFilter\StandardTest does not run » Drupal\editor\Tests\EditorXssFilter\StandardTest does not run and fails on PHP 5.4

So based on info I have just heard from Wim.

Wim Leers’s picture

Title: Drupal\editor\Tests\EditorXssFilter\StandardTest does not run and fails on PHP 5.4 » Drupal\editor\Tests\EditorXssFilter\StandardTest does not run
Status: Needs review » Reviewed & tested by the community
Issue tags: +Quick fix
Related issues: +#2192997: Drupal\editor\Tests\EditorXssFilter\StandardTest has one failure on PHP 5.4, +#2099741: Protect WYSIWYG Editors from XSS Without Destroying User Data

Great catch! Manually tested the patch, this indeed fixes not being able to run the test from the UI.

The test failure you're mentioning only happens on PHP 5.4, so let's fix that in another issue: #2192997: Drupal\editor\Tests\EditorXssFilter\StandardTest has one failure on PHP 5.4.

dawehner’s picture

Note: we should comment out the php 5.4 failure for now, see #2193023: EditorXssFilter/StandardTest::dataset #25 fails on php 5.4 ...

Wim Leers’s picture

#4: Thanks!

damiankloip’s picture

OK, so let's fix it so we get the test here and worry about that there. So do we know what the actual source of the failure is?

Wim Leers’s picture

damiankloip’s picture

Ok, great. Now we just need this so it actually runs properly :)

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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