#2210197: Remove public access to Extension::$type, ::$name, ::$filename introduced some ugly, temporary code to work around Drush on testbots. 5 months later, Drush was hopefully updated.

#2228093: Modernize theme initialization eliminated the magic overloading of theme info properties on Extension objects (replaced by a dedicated ActiveTheme domain object).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, extension.cleanup.0.patch, failed testing.

sun’s picture

Hm. The magically overloaded theme extension properties still appear to be used:

Undefined property: Drupal\Core\Extension\Extension::$info

Drupal\Core\Extension\ThemeHandler->addTheme(Object)
Drupal\Core\Extension\ThemeHandler->listInfo()
list_themes()
_drupal_maintenance_theme()
drupal_maintenance_theme()
drupal_flush_all_caches()

The reason is:

#2228093: Modernize theme initialization only cleaned up theme initialization. However, ThemeHandler::rebuildThemeData() is unrelated to initialization and still operates on Extension objects only.

Given that there is a use-case for instantiating a Theme domain object (including base themes) within that process, we should probably rename the ActiveTheme class into just Theme. — No change in behavior. It only doesn't have to be currently active theme, you can instantiate it for any theme.


However, the good news is that Drush has indeed been updated on testbots, so the other public property workarounds in the Extension class are no longer necessary. :-)

sun’s picture

Status: Needs work » Needs review
FileSize
3.97 KB
1.12 KB

In turn, let's defer that theme data rebuilding clean-up to a separate issue.

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Looks straightforward

dawehner’s picture

Perfect in its limited scope!

sun’s picture

Created #2326407: Remove magic property overloading from Extension class for cleaning up the serialization methods + refactoring the affected theme system functionality.

sun’s picture

FWIW, this cleanup should not require a change notice.

The Extension class was only introduced in D8. The workarounds removed here only had to be added for Drush compatibility (on testbots). The Extension class has proper methods to retrieve all values of interest already.

No code was supposed to use the temporarily added public properties. All of them were annotated with crystal clear @todos.

  • webchick committed 091d092 on 8.0.x
    Issue #2325689 by sun: Clean up temporary Extension class workarounds.
    
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Nice clean-up.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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