Problem/Motivation

#2454447: Split Utility\String class to support PHP 7 (String is a reserved word) deprecated all the functions in Utility/String. String::decodeEntities() was moved to Html::decodeEntities(). Since String class is deprecated but Core still uses String::decodeEntities() so this issue aims to make use of Html::decodeEntities() in all such places.

Proposed resolution

Move all usages of String::decodeEntities() to Html::decodeEntities()

Remaining tasks

  • Patch
  • Review

User interface changes

None

API changes

None

CommentFileSizeAuthor
#27 2457781-decodeentities-27.patch26.52 KBstefan.r
#25 2457781-decodeentities-24.patch26.01 KBstefan.r
#18 use_utility_html_class-2457781-11.patch24.84 KBhussainweb
#14 use_utility_html_class-2457781-14.patch25.73 KBAjitS
#14 interdiff.txt918 bytesAjitS
#11 use_utility_html_class-2457781-11.patch24.84 KBprateekMehta
#8 use_utility_html_class-2457781-8.patch23.02 KBprateekMehta
#4 use_utility_html_class-2457781-3.patch23.09 KBprateekMehta
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Parent issue: » #2454447: Split Utility\String class to support PHP 7 (String is a reserved word)
Related issues: -#2454447: Split Utility\String class to support PHP 7 (String is a reserved word)
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: +Novice
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: +@deprecated
prateekMehta’s picture

prateekMehta CreditAttribution: prateekMehta commented
FileSize
use_utility_html_class-2457781-3.patch23.09 KB

Moving all usages of String::decodeEntities() to Html::decodeEntities()
replacing Drupal\Component\Utility\String::decodeEntities() Drupal\Component\Utility\Html::decodeEntities() or
Adding Drupal\Component\Utility\Html::decodeEntities() wherever required.

prateekMehta’s picture

prateekMehta CreditAttribution: prateekMehta commented
Status: Active » Needs review
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

This looks great, let's see whether it passes.

Status: Needs review » Needs work

The last submitted patch, 4: use_utility_html_class-2457781-3.patch, failed testing.

prateekMehta’s picture

prateekMehta CreditAttribution: prateekMehta commented
FileSize
use_utility_html_class-2457781-8.patch23.02 KB

Had accidentally used Drupal\Component\Utility\Html twice in a couple of files,
removing it.

prateekMehta’s picture

prateekMehta CreditAttribution: prateekMehta commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 8: use_utility_html_class-2457781-8.patch, failed testing.

prateekMehta’s picture

prateekMehta CreditAttribution: prateekMehta commented
FileSize
use_utility_html_class-2457781-11.patch24.84 KB

Replacing at a few other places,sorry for negligence.

prateekMehta’s picture

prateekMehta CreditAttribution: prateekMehta commented
Status: Needs work » Needs review
dpopdan’s picture

dpopdan CreditAttribution: dpopdan commented
Status: Needs review » Needs work

Looks good to me, just please remove decodeEntities() from Drupal\Component\Utility\String.

AjitS’s picture

AjitS
he/him
Primary language Marathi
Location Pune
CreditAttribution: AjitS at QED42 commented
Status: Needs work » Needs review
FileSize
interdiff.txt918 bytes
use_utility_html_class-2457781-14.patch25.73 KB

Removing the deprecated function as suggested in #13.

dpopdan’s picture

dpopdan CreditAttribution: dpopdan commented
Status: Needs review » Reviewed & tested by the community

Thanks !

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/lib/Drupal/Component/Utility/String.php
@@ -32,22 +32,6 @@ public static function checkPlain($text) {
   /**
-   * Decodes all HTML entities including numerical ones to regular UTF-8 bytes.
-   *
-   * @param string $text
-   *   The text to decode entities in.
-   *
-   * @return string
-   *   The input $text, with all HTML entities decoded once.
-   *
-   * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.
-   *   Use \Drupal\Component\Utility\Html::decodeEntities() instead.
-   */
-  public static function decodeEntities($text) {
-    return Html::decodeEntities($text);
-  }

Let's remove this in a separate issue. Makes rollbacks and managing the rtbc queue easier. Also please attached this issue to the change record.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented

In fact we should should have a single issue to remove the string class that will go in after all the conversions are done.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
Status: Needs work » Reviewed & tested by the community
FileSize
use_utility_html_class-2457781-11.patch24.84 KB

I believe that was what was in patch in #11. Uploading here again. Setting to RTBC. Updating change record at https://www.drupal.org/node/2457593.

EDIT: Fixed link to change record.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

Created follow up at #2458387: Remove Utility\String class.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Parent issue: #2454447: Split Utility\String class to support PHP 7 (String is a reserved word) » #2454439: [META] Support PHP 7

Changing parent, so all these issues are visible in the php7 meta issue.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Unsurprisingly, this needs a re-roll after #2457887: Use Utility\SafeMarkup class instead of Utility\String for placeholder(), checkPlain(),format() functions.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
php7-html-utility-2457781-22.patch24.84 KB

Didn't get any merge conflicts in re-roll. Was that expected?

Status: Needs review » Needs work

The last submitted patch, 22: php7-html-utility-2457781-22.patch, failed testing.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper commented

Helps if I update 8.0.x first. :-(

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
FileSize
2457781-decodeentities-24.patch26.01 KB

You may have forgotten to pull? :)

Let's see if this passes tests...

Status: Needs review » Needs work

The last submitted patch, 25: 2457781-decodeentities-24.patch, failed testing.

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented
Status: Needs work » Needs review
FileSize
2457781-decodeentities-27.patch26.52 KB
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Needs review » Reviewed & tested by the community

Checked, all converted and no "use" left

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Priority: Normal » Critical
Status: Reviewed & tested by the community » Fixed

This should be critical since it's parent task is. Committed f3b8d07 and pushed to 8.0.x. Thanks!

  • alexpott committed f3b8d07 on 8.0.x 
    Issue #2457781 by prateekMehta, stefan.r, AjitS, hussainweb: Use Utility...

Status: Fixed » Closed (fixed)

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