Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task, because this removes usages of a previously-deprecated function.
Prioritized changes Removing deprecated code is on the list of prioritized changes.

Function drupal_theme_access is deprecated. Patch attached removes all usages of this function.

CommentFileSizeAuthor
#11 2421661-11.patch1.78 KBmiteshmap
#2 2421661-2.patch1.89 KBjeroent
remove_usages_of_drupal_theme_access.patch1.89 KBjeroent

Comments

Status: Needs review » Needs work

The last submitted patch, remove_usages_of_drupal_theme_access.patch, failed testing.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
StatusFileSize
new 2421661-2.patch1.89 KB

...

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs work » Needs review
miteshmap’s picture

miteshmap
Primary language Gujarati
commented

Patch applied correctly, Only two call occurrence found in 8.0.x , on these two files,
core/modules/system/src/Tests/Theme/ThemeTest.php
core/modules/system/system.module

cilefen’s picture

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

This looks good as described in #4. All implementations are removed.

cilefen’s picture

cilefen commented

@JeroenT or @developermitesh: Could you add a beta evaluation to the issue summary? That will help the committers.

miteshmap’s picture

miteshmap
Primary language Gujarati
commented
Issue summary: View changes
cilefen’s picture

cilefen commented
Issue summary: View changes
alexpott’s picture

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

Let's remove _system_themes_access here too - it looks like it is not used either.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented 
+++ b/core/modules/system/src/Tests/Theme/ThemeTest.php
@@ -208,11 +208,11 @@ function testListThemes() {
+    // Check if \Drupal::service('access_check.theme')->checkAccess() retrieves
+    // enabled themes properly from ThemeHandlerInterface::listInfo().
+    $this->assertTrue(\Drupal::service('access_check.theme')->checkAccess('test_theme'), 'Installed theme detected');
 
-    $this->assertTrue(drupal_theme_access('test_theme'), 'Enabled theme detected');
+    $this->assertTrue(\Drupal::service('access_check.theme')->checkAccess('test_theme'), 'Enabled theme detected');

We only need one check here :) We only have installed themes now.

miteshmap’s picture

miteshmap
Primary language Gujarati
commented
Status: Needs work » Needs review
StatusFileSize
new 2421661-11.patch1.78 KB

removed _system_themes_access, and other unnecessary usage.

linl’s picture

linl commented
Status: Needs review » Closed (duplicate)

Looks like this was fixed elsewhere #2443817: Remove usage and the function drupal_theme_access()