I won't bother writing an IS for something this trivial: we are passing arguments something truly ugly. Fixed.

CommentFileSizeAuthor
#13 interdiff.txt1.88 KBolli
#13 drupal-2077599-13.patch3 KBolli
#4 2077599_4.patch2.94 KBchx
#2 2077599_2.patch2.83 KBchx
XssSplit.patch12.6 KBchx

Comments

chx’s picture

chx commented
StatusFileSize
new 2077599_2.patch2.83 KB

Ah, that one forgot to flip the tags so it'll fail. Here's another version. Alternatively, we could use __CLASS__ and put a final on the whole enchilada. That would look sane to me...

Status: Needs review » Needs work

The last submitted patch, 2077599_2.patch, failed testing.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new 2077599_4.patch2.94 KB

Eh, visibility.

chx’s picture

chx commented
Title: Xss Split needs to be a class for real » Xss Split argument passing needs to come out of stone age
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/lib/Drupal/Component/Utility/Xss.php
@@ -70,7 +68,11 @@ public static function filter($string, $allowed_tags = array('a', 'em', 'strong'
+    $class = get_called_class();
...
+      return $class::split($matches, $allowed_tags);

Does static not work in the closure? Does get_called_class respect LSB?

chx’s picture

chx commented

static in closure? Check http://3v4l.org/G9uLh -- not before 5.4, no. Regarding LSB, http://3v4l.org/42X2U it works since 5.3.0.

olli’s picture

olli commented
Component: postgresql database » ajax system 
+++ b/core/lib/Drupal/Component/Utility/Xss.php
@@ -106,25 +108,22 @@ public static function filterAdmin($string) {
    *   If $store is TRUE then the array contains the allowed tags.
...
+++ b/core/lib/Drupal/Component/Utility/Xss.php
@@ -106,25 +108,22 @@ public static function filterAdmin($string) {
+   *   If $store is FALSE then the array has one element, the HTML tag to
+   *   process.

Lets remove $store.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Marked #2104337: Don't rely on internal function statics in \Drupal\Component\Utility\Xss::split() duplicate of this

chx’s picture

chx commented
Component: ajax system » other

Sure this is not ajax....

dawehner’s picture

dawehner
Primary language German
commented

We should document why we need this $class part here and we could remove it once we depend on 5.4

chx’s picture

chx commented

Do we really? I mean, 5.4 allows you to bind $tihis to the closure for sure but you'd need the classname of it as you are doing a static call. The only 5.4 win would be that you can run the get class inside the closure, that makes no difference IMO.

olli’s picture

olli commented
Issue summary: View changes
StatusFileSize
new drupal-2077599-13.patch3 KB
new interdiff.txt1.88 KB

How about this?

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

I really like that!

alexpott’s picture

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

Committed ebad2e6 and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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