Problem/Motivation

Remove deprecated code from Utility component

  • \Drupal\Component\Utility\Crypt::randomBytes
  • \Drupal\Component\Utility\Crypt::hashEquals
  • \Drupal\Component\Utility\SafeMarkup
  • \Drupal\Component\Utility\Unicode::getStatus
  • \Drupal\Component\Utility\Unicode::strlen
  • \Drupal\Component\Utility\Unicode::strtoupper
  • \Drupal\Component\Utility\Unicode::strtolower
  • \Drupal\Component\Utility\Unicode::substr
  • \Drupal\Component\Utility\Unicode::caseFlip
  • \Drupal\Component\Utility\Unicode::strpos

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

el7cosmos created an issue. See original summary.

el7cosmos’s picture

Component: other » base system
StatusFileSize
new15.02 KB
el7cosmos’s picture

Status: Active » Needs review
el7cosmos’s picture

Issue summary: View changes
el7cosmos’s picture

Title: Remove Unicode component BC layers » Remove Utility component BC layers
Issue summary: View changes
StatusFileSize
new33.35 KB
el7cosmos’s picture

martin107’s picture

Status: Needs review » Reviewed & tested by the community

@el7cosmos thanks for talking the time to sort all this out...

1) I have visually scanned this patch and all the changes make sense to me.

2) When I search for @trigger or @deprecated in the Utilities directory

All looks clear, the hits in DeprecatedArray should remain.

3) The patch applies

4) The are no additional coding standard errors.

mile23’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch in #5 needs a reroll.

ravi.shankar’s picture

Status: Needs work » Needs review
StatusFileSize
new14.99 KB

I have re-rolled.

martin107’s picture

Issue tags: -Needs reroll
martin107’s picture

longwave’s picture

StatusFileSize
new32.91 KB
new17.92 KB

#9 only covered the Unicode class, this adds Crypt and SafeMarkup.

berdir’s picture

Status: Needs review » Needs work

Needs a reroll, the includes removal removed one method as well I think.

krzysztof domański’s picture

Status: Needs work » Needs review
StatusFileSize
new30.37 KB
krzysztof domański’s picture

Unused class TextWrapper after we delete SafeMarkupTest. However not marked as deprecated. Can we also remove it?

core/tests/Drupal/Tests/Component/Utility/TextWrapper.php

namespace Drupal\Tests\Component\Utility;

/**
 * Used by SafeMarkupTest to test that a class with a __toString() method works.
 */
class TextWrapper {
krzysztof domański’s picture

StatusFileSize
new620 bytes
new30.77 KB
longwave’s picture

Re #15 I think that is fine to delete, it was only ever intended for use with the companion class.,

kristen pol’s picture

Category: Plan » Task
Issue tags: -Drupal 9 readiness

Per a Slack discussion with Gábor Hojtsy regarding usage of D9 tags (Drupal 9, Drupal 9 compatibility, Drupal 9 readiness, etc.), "Drupal 9 compatibility" should be used for contributed projects that need updating and "Drupal 9" was the old tag for D8 issues before the D9 branch was ready. Doing tag cleanup here based on that discussion.

krzysztof domański’s picture

StatusFileSize
new841 bytes
new31.59 KB

Addressed #17.

berdir’s picture

+++ /dev/null
@@ -1,74 +0,0 @@
- *   so that the output can can be themed, escaped, and altered properly.
- *
- * @see https://www.drupal.org/node/2549395
- *
- * @see TwigExtension::escapeFilter()
- * @see twig_render_template()
- * @see sanitization
- * @see theme_render
- */
-class SafeMarkup {

Looking at existing "SafeMarkup" references in core..

We have \Drupal\KernelTests\Component\Utility\SafeMarkupKernelTest for example. We could say that's in for the word "safe markup", but at least the class docblock has an explicit "SafeMarkup" reference. And the only thing it really seems to test is :url-style placeholders. Not sure if renaming a test and test methods is something we want to do, so maybe just the docblock?

Then we have SafeMarkupTestMarkup, a test clas in TwigMarkupInterfaceTest, but there's not really a reference to SafeMarkup, so OK?

longwave’s picture

Raised #3098244: Rename SafeMarkupKernelTest to FormattableMarkupKernelTest to handle the first part. I think that the SafeMarkupTestMarkup test class is OK, it is nothing to do with the SafeMarkup concept.

berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, we have a separate issue for that one badly named test class.

  • catch committed 3447368 on 9.0.x
    Issue #3091447 by Krzysztof Domański, el7cosmos, longwave, ravi.shankar...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 3447368 and pushed to 9.0.x. Thanks!

Status: Fixed » Closed (fixed)

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