When we implemented SafeString we had to cast to string because of JSON encoding. I didn't know about \JsonSerializable interface which would allow us to not string cast.

Beta evaluation: Followup from the critical #2506581: Remove SafeMarkup::set() from Renderer::doRender

CommentFileSizeAuthor
#8 2559969.8.patch10.32 KBalexpott
#8 5-8-interdiff.txt1.59 KBalexpott
#5 2559969.5.patch10.32 KBalexpott
#5 2-5-interdiff.txt1.58 KBalexpott
#2 2559969.2.patch11.9 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2559969.2.patch11.9 KB
catch’s picture

catch
he/him
Primary language English
commented

Damn.

Status: Needs review » Needs work

The last submitted patch, 2: 2559969.2.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 2-5-interdiff.txt1.58 KB
new 2559969.5.patch10.32 KB

So we still need the serializer because serialization is more generic than just JSON.

I think this patch helps people who try to implement an Ajax command using the core framework lose less hair.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Big +1

To test I used simpletest to try out things that I expected would use commands. Including quick edit, views dialog/modals and toolbar all work.

Also reviewed the code as well.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Interesting!

+++ b/core/lib/Drupal/Component/Utility/SafeStringTrait.php
@@ -67,4 +67,14 @@ public function count() {
+   * Returns a representation of the object for use in JSON serialisation.

+++ b/core/lib/Drupal/Core/StringTranslation/TranslationWrapper.php
@@ -118,4 +118,14 @@ public function __sleep() {
+   * Returns a representation of the object for use in JSON serialisation.

+++ b/core/lib/Drupal/Core/Template/Attribute.php
@@ -302,4 +302,14 @@ public function storage() {
+   * Returns a representation of the object for use in JSON serialisation.

Nit: s/serialisation/serialization/ — can be fixed on commit.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 5-8-interdiff.txt1.59 KB
new 2559969.8.patch10.32 KB

Thanks @Wim Leers

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 2ea19c7 on 8.0.x 
    Issue #2559969 by alexpott: Make SafeStringInterface extend \...

Status: Fixed » Closed (fixed)

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